All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: "Thomas Hellström" <thomas.hellstrom@linux.intel.com>
Cc: linux-kernel@vger.kernel.org, stable@vger.kernel.org,
	Peter Zijlstra <peterz@infradead.org>,
	mingo@redhat.com, will@kernel.org
Subject: Re: [PATCH AUTOSEL 6.12 3/5] locking/ww_mutex: Adjust to lockdep nest_lock requirements
Date: Mon, 25 Nov 2024 08:34:15 -0500	[thread overview]
Message-ID: <Z0R81y3MDp4xxMmg@sashalap> (raw)
In-Reply-To: <65ace546a7d55c2d6170ca88a48d3de402db2645.camel@linux.intel.com>

On Mon, Nov 25, 2024 at 11:06:38AM +0100, Thomas Hellström wrote:
>On Sun, 2024-11-24 at 07:46 -0500, Sasha Levin wrote:
>> From: Thomas Hellström <thomas.hellstrom@linux.intel.com>
>>
>> [ Upstream commit 823a566221a5639f6c69424897218e5d6431a970 ]
>>
>> When using mutex_acquire_nest() with a nest_lock, lockdep refcounts
>> the
>> number of acquired lockdep_maps of mutexes of the same class, and
>> also
>> keeps a pointer to the first acquired lockdep_map of a class. That
>> pointer
>> is then used for various comparison-, printing- and checking
>> purposes,
>> but there is no mechanism to actively ensure that lockdep_map stays
>> in
>> memory. Instead, a warning is printed if the lockdep_map is freed and
>> there are still held locks of the same lock class, even if the
>> lockdep_map
>> itself has been released.
>>
>> In the context of WW/WD transactions that means that if a user
>> unlocks
>> and frees a ww_mutex from within an ongoing ww transaction, and that
>> mutex happens to be the first ww_mutex grabbed in the transaction,
>> such a warning is printed and there might be a risk of a UAF.
>>
>> Note that this is only problem when lockdep is enabled and affects
>> only
>> dereferences of struct lockdep_map.
>>
>> Adjust to this by adding a fake lockdep_map to the acquired context
>> and
>> make sure it is the first acquired lockdep map of the associated
>> ww_mutex class. Then hold it for the duration of the WW/WD
>> transaction.
>>
>> This has the side effect that trying to lock a ww mutex *without* a
>> ww_acquire_context but where a such context has been acquire, we'd
>> see
>> a lockdep splat. The test-ww_mutex.c selftest attempts to do that, so
>> modify that particular test to not acquire a ww_acquire_context if it
>> is not going to be used.
>>
>> Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
>> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
>> Link:
>> https://lkml.kernel.org/r/20241009092031.6356-1-thomas.hellstrom@linux.intel.com
>> Signed-off-by: Sasha Levin <sashal@kernel.org>
>
>It looks like the last version was not picked for this patch.
>
>https://lore.kernel.org/all/172918113162.1279674.6570518059490493206@2413ebb6fbb6/T/
>
>The version in this autosel patch regresses the locking api selftests
>and should not be backported. Same for the corresponding backports for
>6.11 and 6.6. Let me know if I should reply separately to those.

This is what ended up landing upstream...

I can drop it from the autosel queue, but if this has issues then you
should also fix it up upstream.

-- 
Thanks,
Sasha

  reply	other threads:[~2024-11-25 13:34 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-11-24 12:46 [PATCH AUTOSEL 6.12 1/5] uprobes: sanitiize xol_free_insn_slot() Sasha Levin
2024-11-24 12:46 ` [PATCH AUTOSEL 6.12 2/5] perf/x86/amd: Warn only on new bits set Sasha Levin
2024-11-24 12:46 ` [PATCH AUTOSEL 6.12 3/5] locking/ww_mutex: Adjust to lockdep nest_lock requirements Sasha Levin
2024-11-25 10:06   ` Thomas Hellström
2024-11-25 13:34     ` Sasha Levin [this message]
2024-11-24 12:46 ` [PATCH AUTOSEL 6.12 4/5] cleanup: Adjust scoped_guard() macros to avoid potential warning Sasha Levin
2024-11-24 12:46 ` [PATCH AUTOSEL 6.12 5/5] timekeeping: Always check for negative motion Sasha Levin
2024-11-24 13:13 ` [PATCH AUTOSEL 6.12 1/5] uprobes: sanitiize xol_free_insn_slot() Oleg Nesterov
2024-11-24 14:15   ` Sasha Levin
2024-11-24 14:36     ` Oleg Nesterov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Z0R81y3MDp4xxMmg@sashalap \
    --to=sashal@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=stable@vger.kernel.org \
    --cc=thomas.hellstrom@linux.intel.com \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.