Re: [PATCH v2] scsi: address leak in scsi_setup_discard_cmnd error path (was: Re: scsi: unify the error handling of the prep functions)

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

 



On Thu, Jul 08 2010 at 12:17am -0400,
FUJITA Tomonori <fujita.tomonori@xxxxxxxxxxxxx> wrote:

> On Tue, 6 Jul 2010 09:59:58 -0400
> Mike Snitzer <snitzer@xxxxxxxxxx> wrote:
> 
> > Here is a minimalist patch that 1) removes the discard request's page
> > leak 2) preserves the prep cleanup rules covered above.  Fixing the leak
> > is a priority, introducing additional error path code sharing/cleanup is
> > secondary and can come later.
> 
> This patch doesn't work. I hit kernel oops since now this patch frees
> a discard page twice.

I load tested my v2 patch quite a bit but didn't hit the oops.  Guess
you're just lucky ;)

> If scsi_init_io hits BLKPREP_DEFER (e.g. due to scsi_init_sgtable),
> scsi_setup_blk_pc_cmnd calls scsi_unprep_request. So sd_unprep_fn
> frees a page. Then, scsi_setup_discard_cmnd frees the same page again.

OK, thanks for catching this.

> From: FUJITA Tomonori <fujita.tomonori@xxxxxxxxxxxxx>
> Subject: [PATCH] scsi: fix discard page leak
> 
> We leak a page allocated for discard on some error conditions
> (e.g. scsi_prep_state_check returns BLKPREP_DEFER in
> scsi_setup_blk_pc_cmnd).
> 
> We unprep on requests that weren't prepped in the error path of
> scsi_init_io. It makes the error path to clean up scsi commands messy.
> 
> Let's strictly apply the rule that we can't unprep on a request that
> wasn't prepped.
> 
> Calling just scsi_put_command() in the error path of scsi_init_io() is
> enough. We don't set REQ_DONTPREP yet.
> 
> scsi_setup_discard_cmnd can safely free a page on the error case with
> the above rule.
> 
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@xxxxxxxxxxxxx>

Acked-by: Mike Snitzer <snitzer@xxxxxxxxxx>

Jens, it'd be great if we could get this fix in now.

--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel


[Index of Archives]     [DM Crypt]     [Fedora Desktop]     [ATA RAID]     [Fedora Marketing]     [Fedora Packaging]     [Fedora SELinux]     [Yosemite Discussion]     [KDE Users]     [Fedora Docs]

  Powered by Linux