Skip to content

Commit

Permalink
scsi: fix discard page leak
Browse files Browse the repository at this point in the history
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@lab.ntt.co.jp>
Signed-off-by: Jens Axboe <jaxboe@fusionio.com>
  • Loading branch information
FUJITA Tomonori authored and Jens Axboe committed Aug 7, 2010
1 parent 9e09438 commit 610a634
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 7 deletions.
7 changes: 2 additions & 5 deletions drivers/scsi/scsi_lib.c
Original file line number Diff line number Diff line change
Expand Up @@ -1011,11 +1011,8 @@ int scsi_init_io(struct scsi_cmnd *cmd, gfp_t gfp_mask)

err_exit:
scsi_release_buffers(cmd);
if (error == BLKPREP_KILL)
scsi_put_command(cmd);
else /* BLKPREP_DEFER */
scsi_unprep_request(cmd->request);

scsi_put_command(cmd);
cmd->request->special = NULL;
return error;
}
EXPORT_SYMBOL(scsi_init_io);
Expand Down
10 changes: 8 additions & 2 deletions drivers/scsi/sd.c
Original file line number Diff line number Diff line change
Expand Up @@ -468,6 +468,10 @@ static int scsi_setup_discard_cmnd(struct scsi_device *sdp, struct request *rq)
blk_add_request_payload(rq, page, len);
ret = scsi_setup_blk_pc_cmnd(sdp, rq);
rq->buffer = page_address(page);
if (ret != BLKPREP_OK) {
__free_page(page);
rq->buffer = NULL;
}
return ret;
}

Expand All @@ -485,8 +489,10 @@ static int scsi_setup_flush_cmnd(struct scsi_device *sdp, struct request *rq)

static void sd_unprep_fn(struct request_queue *q, struct request *rq)
{
if (rq->cmd_flags & REQ_DISCARD)
__free_page(virt_to_page(rq->buffer));
if (rq->cmd_flags & REQ_DISCARD) {
free_page((unsigned long)rq->buffer);
rq->buffer = NULL;
}
}

/**
Expand Down

0 comments on commit 610a634

Please sign in to comment.