[-nect,RFC,v2,0/2] block: fix uaf in bd_link_disk_holder()

Message

Yu Kuai Oct. 20, 2022, 1:20 p.m. UTC

Changes in v2:
 - in order to know when bd_holder_dir will be freed, instead of changing
kobject_put(), add a new field in block_device.

Yu Kuai (2):
  block: add helpers for bd_holder_dir refcount management
  block: fix uaf for bd_holder_dir

 block/blk.h               |  3 +++
 block/genhd.c             | 33 ++++++++++++++++++++++++++++++++-
 block/holder.c            | 19 +++++++++++++------
 block/partitions/core.c   |  5 ++++-
 include/linux/blk_types.h |  1 +
 5 files changed, 53 insertions(+), 8 deletions(-)

Comments

Christoph Hellwig Oct. 20, 2022, 4:47 p.m. UTC | #1

As mentioned before I don't think we should make this even more
crufty in the block layer.  See the series I just sent to move it int
dm.

Yu Kuai Oct. 21, 2022, 3:15 a.m. UTC | #2

Hi,

在 2022/10/21 0:47, Christoph Hellwig 写道:
> As mentioned before I don't think we should make this even more
> crufty in the block layer.  See the series I just sent to move it int
> dm.

It seems we had some misunderstanding, the problem I tried to fix here
should not just related to dm, but all the caller of
bd_link_disk_holder().

Thanks,
Kuai
> 
> .
>

Yu Kuai Oct. 26, 2022, 11:16 a.m. UTC | #3

Hi, Christoph

在 2022/10/21 11:15, Yu Kuai 写道:
> Hi,
> 
> 在 2022/10/21 0:47, Christoph Hellwig 写道:
>> As mentioned before I don't think we should make this even more
>> crufty in the block layer.  See the series I just sent to move it int
>> dm.
> 
> It seems we had some misunderstanding, the problem I tried to fix here
> should not just related to dm, but all the caller of
> bd_link_disk_holder().

Any suggestions about how to fix this problem?

Thanks,
Kuai
> 
> Thanks,
> Kuai
>>
>> .
>>
> 
> .
>

Christoph Hellwig Oct. 30, 2022, 3:30 p.m. UTC | #4

On Fri, Oct 21, 2022 at 11:15:34AM +0800, Yu Kuai wrote:
> Hi,
>
> 在 2022/10/21 0:47, Christoph Hellwig 写道:
>> As mentioned before I don't think we should make this even more
>> crufty in the block layer.  See the series I just sent to move it int
>> dm.
>
> It seems we had some misunderstanding, the problem I tried to fix here
> should not just related to dm, but all the caller of
> bd_link_disk_holder().

As far as I can tell the problem was just that patch 1 in my series blows
away the bd_holder_dir pointer in part0 on del_gendisk.  Each holder
actually holds a reference to the kobject, so the memory for it is
still valid, it's just that the pointer got cleared.  I'll send a v2
in a bit.

Yu Kuai Oct. 31, 2022, 1:08 a.m. UTC | #5

Hi, Christoph

在 2022/10/30 23:30, Christoph Hellwig 写道:
> On Fri, Oct 21, 2022 at 11:15:34AM +0800, Yu Kuai wrote:
>> Hi,
>>
>> 在 2022/10/21 0:47, Christoph Hellwig 写道:
>>> As mentioned before I don't think we should make this even more
>>> crufty in the block layer.  See the series I just sent to move it int
>>> dm.
>>
>> It seems we had some misunderstanding, the problem I tried to fix here
>> should not just related to dm, but all the caller of
>> bd_link_disk_holder().
> 
> As far as I can tell the problem was just that patch 1 in my series blows
> away the bd_holder_dir pointer in part0 on del_gendisk.  Each holder
> actually holds a reference to the kobject, so the memory for it is
> still valid, it's just that the pointer got cleared.  I'll send a v2
> in a bit.

This is not the real case. In bd_link_disk_hoder(), bd_hodler_dir is
accessed first by add_symlink(), and then reference is grabed later.
The reference should be grabed before bd_holder_dir is accessed, like
what I try to do in patch 2.

Thanks,
Kuai
> 
> .
>