diff mbox series

[SRU,Xenial,CVE-2018-20856] block: blk_init_allocated_queue() set q->fq as NULL in the fail case

Message ID 20190802171128.10484-1-connor.kuehl@canonical.com
State New
Headers show
Series [SRU,Xenial,CVE-2018-20856] block: blk_init_allocated_queue() set q->fq as NULL in the fail case | expand

Commit Message

Connor Kuehl Aug. 2, 2019, 5:11 p.m. UTC 
From: xiao jin <jin.xiao@intel.com>

CVE-2018-20856

We find the memory use-after-free issue in __blk_drain_queue()
on the kernel 4.14. After read the latest kernel 4.18-rc6 we
think it has the same problem.

Memory is allocated for q->fq in the blk_init_allocated_queue().
If the elevator init function called with error return, it will
run into the fail case to free the q->fq.

Then the __blk_drain_queue() uses the same memory after the free
of the q->fq, it will lead to the unpredictable event.

The patch is to set q->fq as NULL in the fail case of
blk_init_allocated_queue().

Fixes: commit 7c94e1c157a2 ("block: introduce blk_flush_queue to drive flush machinery")
Cc: <stable@vger.kernel.org>
Reviewed-by: Ming Lei <ming.lei@redhat.com>
Reviewed-by: Bart Van Assche <bart.vanassche@wdc.com>
Signed-off-by: xiao jin <jin.xiao@intel.com>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
(backported from commit 54648cf1ec2d7f4b6a71767799c45676a138ca24)
[ Connor Kuehl: had to place the line from the patch in manually since
  the patch context disagreed with what the routine looks like now
  (different label, different return statement). Barely more involved
  than an offset adjustment. ]
Signed-off-by: Connor Kuehl <connor.kuehl@canonical.com>
---
 block/blk-core.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Tyler Hicks Aug. 9, 2019, 2:55 p.m. UTC | #1 
On 2019-08-02 10:11:28, Connor Kuehl wrote:
> From: xiao jin <jin.xiao@intel.com>
> 
> CVE-2018-20856
> 
> We find the memory use-after-free issue in __blk_drain_queue()
> on the kernel 4.14. After read the latest kernel 4.18-rc6 we
> think it has the same problem.
> 
> Memory is allocated for q->fq in the blk_init_allocated_queue().
> If the elevator init function called with error return, it will
> run into the fail case to free the q->fq.
> 
> Then the __blk_drain_queue() uses the same memory after the free
> of the q->fq, it will lead to the unpredictable event.
> 
> The patch is to set q->fq as NULL in the fail case of
> blk_init_allocated_queue().
> 
> Fixes: commit 7c94e1c157a2 ("block: introduce blk_flush_queue to drive flush machinery")
> Cc: <stable@vger.kernel.org>
> Reviewed-by: Ming Lei <ming.lei@redhat.com>
> Reviewed-by: Bart Van Assche <bart.vanassche@wdc.com>
> Signed-off-by: xiao jin <jin.xiao@intel.com>
> Signed-off-by: Jens Axboe <axboe@kernel.dk>
> (backported from commit 54648cf1ec2d7f4b6a71767799c45676a138ca24)
> [ Connor Kuehl: had to place the line from the patch in manually since
>   the patch context disagreed with what the routine looks like now
>   (different label, different return statement). Barely more involved
>   than an offset adjustment. ]
> Signed-off-by: Connor Kuehl <connor.kuehl@canonical.com>

Acked-by: Tyler Hicks <tyhicks@canonical.com>

Thanks!

Tyler

> ---
>  block/blk-core.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/block/blk-core.c b/block/blk-core.c
> index f83b9c2b6a14..a3e1f4fcb2e5 100644
> --- a/block/blk-core.c
> +++ b/block/blk-core.c
> @@ -861,6 +861,7 @@ blk_init_allocated_queue(struct request_queue *q, request_fn_proc *rfn,
>  
>  fail:
>  	blk_free_flush_queue(q->fq);
> +	q->fq = NULL;
>  	return NULL;
>  }
>  EXPORT_SYMBOL(blk_init_allocated_queue);
> -- 
> 2.20.1
> 
> 
> -- 
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
Khalid Elmously Aug. 12, 2019, 4:12 a.m. UTC | #2 
On 2019-08-02 10:11:28 , Connor Kuehl wrote:
> From: xiao jin <jin.xiao@intel.com>
> 
> CVE-2018-20856
> 
> We find the memory use-after-free issue in __blk_drain_queue()
> on the kernel 4.14. After read the latest kernel 4.18-rc6 we
> think it has the same problem.
> 
> Memory is allocated for q->fq in the blk_init_allocated_queue().
> If the elevator init function called with error return, it will
> run into the fail case to free the q->fq.
> 
> Then the __blk_drain_queue() uses the same memory after the free
> of the q->fq, it will lead to the unpredictable event.
> 
> The patch is to set q->fq as NULL in the fail case of
> blk_init_allocated_queue().
> 
> Fixes: commit 7c94e1c157a2 ("block: introduce blk_flush_queue to drive flush machinery")
> Cc: <stable@vger.kernel.org>
> Reviewed-by: Ming Lei <ming.lei@redhat.com>
> Reviewed-by: Bart Van Assche <bart.vanassche@wdc.com>
> Signed-off-by: xiao jin <jin.xiao@intel.com>
> Signed-off-by: Jens Axboe <axboe@kernel.dk>
> (backported from commit 54648cf1ec2d7f4b6a71767799c45676a138ca24)
> [ Connor Kuehl: had to place the line from the patch in manually since
>   the patch context disagreed with what the routine looks like now
>   (different label, different return statement). Barely more involved
>   than an offset adjustment. ]
> Signed-off-by: Connor Kuehl <connor.kuehl@canonical.com>
> ---
>  block/blk-core.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/block/blk-core.c b/block/blk-core.c
> index f83b9c2b6a14..a3e1f4fcb2e5 100644
> --- a/block/blk-core.c
> +++ b/block/blk-core.c
> @@ -861,6 +861,7 @@ blk_init_allocated_queue(struct request_queue *q, request_fn_proc *rfn,
>  
>  fail:
>  	blk_free_flush_queue(q->fq);
> +	q->fq = NULL;
>  	return NULL;
>  }
>  EXPORT_SYMBOL(blk_init_allocated_queue);
> -- 
> 2.20.1
> 
> 
> -- 
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
Khalid Elmously Aug. 12, 2019, 4:13 a.m. UTC | #3 
On 2019-08-02 10:11:28 , Connor Kuehl wrote:
> From: xiao jin <jin.xiao@intel.com>
> 
> CVE-2018-20856
> 
> We find the memory use-after-free issue in __blk_drain_queue()
> on the kernel 4.14. After read the latest kernel 4.18-rc6 we
> think it has the same problem.
> 
> Memory is allocated for q->fq in the blk_init_allocated_queue().
> If the elevator init function called with error return, it will
> run into the fail case to free the q->fq.
> 
> Then the __blk_drain_queue() uses the same memory after the free
> of the q->fq, it will lead to the unpredictable event.
> 
> The patch is to set q->fq as NULL in the fail case of
> blk_init_allocated_queue().
> 
> Fixes: commit 7c94e1c157a2 ("block: introduce blk_flush_queue to drive flush machinery")
> Cc: <stable@vger.kernel.org>
> Reviewed-by: Ming Lei <ming.lei@redhat.com>
> Reviewed-by: Bart Van Assche <bart.vanassche@wdc.com>
> Signed-off-by: xiao jin <jin.xiao@intel.com>
> Signed-off-by: Jens Axboe <axboe@kernel.dk>
> (backported from commit 54648cf1ec2d7f4b6a71767799c45676a138ca24)
> [ Connor Kuehl: had to place the line from the patch in manually since
>   the patch context disagreed with what the routine looks like now
>   (different label, different return statement). Barely more involved
>   than an offset adjustment. ]
> Signed-off-by: Connor Kuehl <connor.kuehl@canonical.com>
> ---
>  block/blk-core.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/block/blk-core.c b/block/blk-core.c
> index f83b9c2b6a14..a3e1f4fcb2e5 100644
> --- a/block/blk-core.c
> +++ b/block/blk-core.c
> @@ -861,6 +861,7 @@ blk_init_allocated_queue(struct request_queue *q, request_fn_proc *rfn,
>  
>  fail:
>  	blk_free_flush_queue(q->fq);
> +	q->fq = NULL;
>  	return NULL;
>  }
>  EXPORT_SYMBOL(blk_init_allocated_queue);
> -- 
> 2.20.1
> 
> 
> -- 
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
diff mbox series

Patch

diff --git a/block/blk-core.c b/block/blk-core.c
index f83b9c2b6a14..a3e1f4fcb2e5 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -861,6 +861,7 @@  blk_init_allocated_queue(struct request_queue *q, request_fn_proc *rfn,
 
 fail:
 	blk_free_flush_queue(q->fq);
+	q->fq = NULL;
 	return NULL;
 }
 EXPORT_SYMBOL(blk_init_allocated_queue);