Re: [PATCH] blk-cgroup: delay calling blkcg_exit_disk until disk_release

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Feb 10, 2023 at 01:05:11PM +0800, Ming Lei wrote:
> On Wed, Feb 08, 2023 at 07:35:14AM +0100, Christoph Hellwig wrote:
> > While del_gendisk ensures there is no outstanding I/O on the queue,
> > it can't prevent block layer users from building new I/O.
> > 
> > This leads to a NULL ->root_blkg reference in bio_associate_blkg when
> > allocating a new bio on a shut down file system.  Delay freeing the
> > blk-cgroup subsystems from del_gendisk until disk_release to make
> > sure the blkg and throttle information is still avaіlable for bio
> > submitters, even if those bios will immediately fail.
> > 
> > This now can cause a case where disk_release is called on a disk
> > that hasn't been added.  That's mostly harmless, except for a case
> > in blk_throttl_exit that now needs to check for a NULL ->td pointer.
> > 
> > Fixes: 178fa7d49815 ("blk-cgroup: delay blk-cgroup initialization until add_disk")
> > Reported-by: Ming Lei <ming.lei@xxxxxxxxxx>
> > Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> 
> hammmmmm, this patch actually causes bigger trouble.
> 
> After commit 84d7d462b16d ("blk-cgroup: pin the gendisk in struct blkcg_gq"),
> blkcg_gq instance grabs disk's reference, so moving blkcg_exit_disk
> into disk_release() just causes reference cross-dependency, both are
> leaked.

Hi Christoph & Jens,

This issue is a bit serious, both blkg & disk & request_queue are leaked by
commit c43332fe028c ("blk-cgroup: delay calling blkcg_exit_disk until disk_release").

Can we solve it before merging for-6.3/block into v6.3-rc1?

Thanks,
Ming




[Index of Archives]     [Linux RAID]     [Linux SCSI]     [Linux ATA RAID]     [IDE]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Device Mapper]

  Powered by Linux