From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id D129BC433F5 for ; Wed, 6 Apr 2022 11:51:08 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 472DD6B0072; Wed, 6 Apr 2022 07:50:58 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 422086B0073; Wed, 6 Apr 2022 07:50:58 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 29CB86B0074; Wed, 6 Apr 2022 07:50:58 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (relay.a.hostedemail.com [64.99.140.24]) by kanga.kvack.org (Postfix) with ESMTP id 14EAA6B0072 for ; Wed, 6 Apr 2022 07:50:58 -0400 (EDT) Received: from smtpin07.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id C2C2E60C4A for ; Wed, 6 Apr 2022 11:50:47 +0000 (UTC) X-FDA: 79326287334.07.27BA610 Received: from mail-pl1-f170.google.com (mail-pl1-f170.google.com [209.85.214.170]) by imf10.hostedemail.com (Postfix) with ESMTP id 3E0B5C001F for ; Wed, 6 Apr 2022 11:50:47 +0000 (UTC) Received: by mail-pl1-f170.google.com with SMTP id o20so1703399pla.13 for ; Wed, 06 Apr 2022 04:50:47 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=LexSt4aevETZp9OYJUcU75mB3szv4z4iC4bK+kg4wrw=; b=RLEQBPKPm2Y+qKo1Y5nutRYAEJ5mnhdcakkUzgDrH6n5zbU8poEo1Z+Au767FbLKdH rvPcPSkwsQ8lKYXiANnZG5tIguNgo7izrATjlgpWdTNCpSSHWLKKoVj/xiOZX9gz7oOx KtdbBj2gm9IRc7h5vxNl3gLzLRkionLb9hsDjNnAlmfQC0yJotz/NuT1N1ymnXj4WGBL EmJSmr1BaLb54QF8IAr8sNYYOBQrPJ0zqFfA0ui5ca68eybQo2zlM8n+DwB8UGgmwacG VMX6uwfOg3TA1HDec392SkH8lOVMSCELlIsgj5Zv5n0JigeMiBY7aVbny8HTP/ksr8Of ZpDg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=LexSt4aevETZp9OYJUcU75mB3szv4z4iC4bK+kg4wrw=; b=LvdIamrfB3zBUCPILrTt2eRG1yJgMQrUGjvFSECG+YXU3i0XhS7RvtmeJJCk+ueRDj qDLgn0SN7BYo7y0EBwKhNc6uE2Rz3JVA69SwzhbVda3ckhNtiufZJUJMeiHpfItsdB/9 PrCiR4Cp1MEx/FqNHz2UVmA8Q/sw/nqalQ3VTd2G4olzzwiBPrxQExAxpKRX4E5s5mEB LpuefZlsJrMrM34BUzZ6D7ZhIAxybwljq55ym/f3qr9Jyuz4tMPTl+Z/7Apvma2g2sON xOQcvROjziN4oofPNAkIV01MrMyTjEv5RLpPCHm4MHuJXKy4MgqfFkX7pb6SqYHOKCcr vDPA== X-Gm-Message-State: AOAM5338R7qIPesz+qA0WePWRC8jMBW8tG3bKiS+PGNlIB0IZZwvT6Ps c7Be0ZTC1hz8Oq9PFjyYCAw= X-Google-Smtp-Source: ABdhPJwXJkv8T7an8WY+DDHwwAghsTTZPZLvCw/9PSzzyUVU7ISrUysJcUH2S+tJX1OZ+QT4k87b8g== X-Received: by 2002:a17:902:c948:b0:156:c07d:8222 with SMTP id i8-20020a170902c94800b00156c07d8222mr8349170pla.30.1649245846123; Wed, 06 Apr 2022 04:50:46 -0700 (PDT) Received: from hyeyoo ([114.29.24.243]) by smtp.gmail.com with ESMTPSA id f16-20020a056a001ad000b004fb358ffe86sm19212686pfv.137.2022.04.06.04.50.40 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 06 Apr 2022 04:50:44 -0700 (PDT) Date: Wed, 6 Apr 2022 20:50:37 +0900 From: Hyeonggon Yoo <42.hyeyoo@gmail.com> To: Marco Elver Cc: Vlastimil Babka , kernel test robot , Oliver Glitta , lkp@lists.01.org, lkp@intel.com, LKML , Imran Khan , Andrey Konovalov , Zhen Lei , Zqiang , linux-mm@kvack.org Subject: Re: [mm/slub] 555b8c8cb3: WARNING:at_lib/stackdepot.c:#stack_depot_fetch Message-ID: References: <20220324095218.GA2108184@odroid> <8368021e-86c3-a93f-b29d-efed02135c41@suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: 3E0B5C001F X-Rspam-User: Authentication-Results: imf10.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=RLEQBPKP; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf10.hostedemail.com: domain of 42.hyeyoo@gmail.com designates 209.85.214.170 as permitted sender) smtp.mailfrom=42.hyeyoo@gmail.com X-Stat-Signature: pjp7ermhexeeresxactz3mp8841t8xtz X-HE-Tag: 1649245847-691583 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Wed, Apr 06, 2022 at 10:34:07AM +0200, Marco Elver wrote: > On Wed, Apr 06, 2022 at 07:47AM +0900, Hyeonggon Yoo wrote: > > On Tue, Apr 05, 2022 at 01:07:53PM +0200, Marco Elver wrote: > > > On Tue, Apr 05, 2022 at 11:00AM +0900, Hyeonggon Yoo wrote: > > > > On Mon, Apr 04, 2022 at 05:18:16PM +0200, Marco Elver wrote: > > > > > On Mon, 4 Apr 2022 at 16:20, Vlastimil Babka wrote: > > > [...] > > > > > > But here we are in mem_dump_obj() -> kmem_dump_obj() -> kmem_obj_info(). > > > > > > Because kmem_valid_obj() returned true, fooled by folio_test_slab() > > > > > > returning true because of the /* Set required slab fields. */ code. > > > > > > Yet the illusion is not perfect and we read garbage instead of a valid > > > > > > stackdepot handle. > > > > > > > > > > > > IMHO we should e.g. add the appropriate is_kfence_address() test into > > > > > > kmem_valid_obj(), to exclude kfence-allocated objects? Sounds much simpler > > > > > > than trying to extend the illusion further to make kmem_dump_obj() work? > > > > > > Instead kfence could add its own specific handler to mem_dump_obj() to print > > > > > > its debugging data? > > > > > > > > > > I think this explanation makes sense! Indeed, KFENCE already records > > > > > allocation stacks internally anyway, so it should be straightforward > > > > > to convince it to just print that. > > > > > > > > > > > > > Thank you both! Yeah the explanation makes sense... thats why KASAN/KCSAN couldn't yield anything -- it was not overwritten. > > > > > > > > I'm writing a fix and will test if the bug disappears. > > > > This may take few days. > > > > > > > I did check the bug is not reproduced after simple fix. (reproduced 0 of 373) > > This approach was right. > > > > > The below should fix it -- I'd like to make kmem_obj_info() do something > > > useful for KFENCE objects. > > > > > > > Agreed. > > > [...] > > > + i = get_stack_skipnr(track->stack_entries, track->num_stack_entries, NULL); > > > + for (j = 0; i < track->num_stack_entries && j < KS_ADDRS_COUNT - 1; ++i, ++j) > > > > why KS_ADDRS_COUNT - 1 instead of KS_ADDRS_COUNT? > > For `kp_stack[j] = NULL` because KFENCE's stack_entries does not have a > NULL-delimiter (we have num_stack_entries). But it seems for kp_stack > it's only added if `j < KS_ADDR_COUNT`, so I've fixed that. > Okay. > > > + kp_stack[j] = (void *)track->stack_entries[i]; > > > + kp_stack[j] = NULL; > [...] > > > + kpp->kp_objp = (void *)meta->addr; > > > + > > > > no need to take meta->lock here? > > Yes, in case state is KFENCE_OBJECT_FREED there could be a race. > dumping object that is freed sounds like a bug? but seems it's better to avoid a race in that case too. > > > + kfence_to_kp_stack(&meta->alloc_track, kpp->kp_stack); > > > + if (meta->state == KFENCE_OBJECT_FREED) > > > + kfence_to_kp_stack(&meta->free_track, kpp->kp_free_stack); > > > + /* get_stack_skipnr() ensures the first entry is outside allocator. */ > > > + kpp->kp_ret = kpp->kp_stack[0]; > > > + > > > + return true; > > > +} > > > > kfence_kmem_obj_info() does not set kp_data_offset. kp_data_offset > > may not be zero when e.g.) mem_dump_obj(&rhp->func); in rcutorture case. > > kp_data_offset is the offset e.g. when SLUB has added a redzone: > > | objp0 = kasan_reset_tag(object); > | #ifdef CONFIG_SLUB_DEBUG > | objp = restore_red_left(s, objp0); > | #else > | objp = objp0; > | #endif > | objnr = obj_to_index(s, slab, objp); > | kpp->kp_data_offset = (unsigned long)((char *)objp0 - (char *)objp); > > In !CONFIG_SLUB_DEBUG and !(s->flags & SLAB_RED_ZONE) cases it's always > 0, and otherwise it's > > `objp0 - restore_red_left(objp0)` == > `object - (object - s->red_left_pad)` == > `s->red_left_pad`. > > This matters if kp_objp is not the object start accessible by the user. > But in the KFENCE case this is always the case so kp_data_offset=0. > Ah, right. confused "pointer offset" with "data offset". it's always 0 in kfence case. > > BTW, I would prefer implementing something like kfence_obj_info() > > (called by kmem_dump_obj() and called instead of kmem_obj_info()) > > for better readability. > > Hmm, I guess that saves us from having to fix up both slab.c/slub.c. But > it makes kmem_obj_info() error-prone to use. What if someone calls > kmem_obj_info() in future somewhere else? That caller then would have to > remember to also call kfence_obj_info(). > Valid point. > I'd prefer fixing it as close to the root-cause (in kmem_obj_info()) to > avoid that. > > What do you prefer? Then what about something like this? kmem_obj_info(object) { if (object is from kfence) __kfence_obj_info() else __kmem_obj_info() } > > > And when mem_dump_obj() is called, I guess it's for debugging purpose. > > I think it would be better to let user know the object is allocated > > from kfence pool. maybe adding if (is_kfence_address(object)) pr_cont(" kfence"); > > in kmem_dump_obj() would be enough? > > We can add that. > > Thanks, > -- Marco -- Thanks, Hyeonggon