Re: [PATCH] dm: fix use-after-free when dm_get_device() widens the open mode

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Low] dm: missing Fixes trailer for commit 86f1152b117a in commit
  message
- [Low] dm: commit message claim about table_device->count contradicts
  code
--

commit 16907af74a028c1489b22d6e8c7c48ae64949725
Author: Ming-Hung Tsai <mtsai@redhat.com>

> dm: fix use-after-free when dm_get_device() widens the open mode
>
> There's an UAF bug in dm_get_device(): when a table construction calls
> dm_get_device() on the same device twice with widen access mode (e.g.,
> read-only first then read-write), the second dm_get_device() calls
> upgrade_mode(), which reopens the device in upgraded mode, making
> previously returned dm_dev* a stale pointer.
>
> Fixing approaches:
> - Add per-mode views (dm_dev_ref) from a per-device dm_dev_internal to
>   multiple table_devices.
> - Keep reusing the dm_dev_ref when the requested mode is included, i.e.,
>   a read-only reference can reuse the read-write opened device within a
>   table.
> - Maintain a canonical block_device pointer in dm_dev_internal for
>   handling the DM_TABLE_DEPS ioctl.
>
> Also tweak the way of mode sharing check in find_ref() to
> "(x & mask) == mask". The result stays the same to the previous
> upgrade_mode() guard "(x | mask) == x".
>
> Pros: keep the relationship between dm_dev_internal and table_device:
> one dm_dev_internal per device, regardless the mode, so the output of
> DMM_TABLE_DEPS ioctl unchanged.
>
> Cons: extra dm_dev_ref allocation.
>
> Note that now the table_device's count not necessarily mean the number
> tables referecing this <dev, mode> pair, since we're allowing multiple
> references within a table.

[Severity: Low]
Is the closing note about table_device->count accurate?  In
dm_get_device(), find_ref() reuses an existing dm_dev_ref whenever its
mode covers the requested mode, so a table still holds at most one
dm_dev_ref (and one reference to the table_device) for any given
<dev, mode> pair, as the comment on struct dm_dev_ref in
drivers/md/dm.h notes.

>
> Signed-off-by: Ming Hung Tsai <mtsai@redhat.com>
> Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
> Cc: stable@vger.kernel.org

[Severity: Low]
Since 86f1152b117a ("dm: allow active and inactive tables to share
dm_devs") changed upgrade_mode() to replace dd->dm_dev and drop the old
table_device reference while earlier targets in the table still held it,
should this include:

  Fixes: 86f1152b117a ("dm: allow active and inactive tables to share dm_devs")

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/7acdd5f5-72bd-1fc2-cf90-26c9c2a16ba8@redhat.com?part=1