diff mbox series

[SRU,Trusty,1/1] scsi: sg: fixup infoleak when using SG_GET_REQUEST_TABLE

Message ID 20180711154525.32498-2-kleber.souza@canonical.com
State New
Headers show
Series Fix for CVE-2017-14991 | expand

Commit Message

Kleber Sacilotto de Souza July 11, 2018, 3:45 p.m. UTC 
From: Hannes Reinecke <hare@suse.de>

When calling SG_GET_REQUEST_TABLE ioctl only a half-filled table is
returned; the remaining part will then contain stale kernel memory
information.  This patch zeroes out the entire table to avoid this
issue.

Signed-off-by: Hannes Reinecke <hare@suse.com>
Reviewed-by: Bart Van Assche <bart.vanassche@wdc.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>

CVE-2017-14991
(backported from commit 3e0097499839e0fe3af380410eababe5a47c4cf9)
Signed-off-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com>
---
 drivers/scsi/sg.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

Comments

Colin Ian King July 11, 2018, 3:46 p.m. UTC | #1 
On 11/07/18 16:45, Kleber Sacilotto de Souza wrote:
> From: Hannes Reinecke <hare@suse.de>
> 
> When calling SG_GET_REQUEST_TABLE ioctl only a half-filled table is
> returned; the remaining part will then contain stale kernel memory
> information.  This patch zeroes out the entire table to avoid this
> issue.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.com>
> Reviewed-by: Bart Van Assche <bart.vanassche@wdc.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Eric Dumazet <edumazet@google.com>
> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
> 
> CVE-2017-14991
> (backported from commit 3e0097499839e0fe3af380410eababe5a47c4cf9)
> Signed-off-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com>
> ---
>  drivers/scsi/sg.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
> index 47b8f7b8b7b4..1c3dd355b317 100644
> --- a/drivers/scsi/sg.c
> +++ b/drivers/scsi/sg.c
> @@ -1003,14 +1003,13 @@ sg_ioctl(struct file *filp, unsigned int cmd_in, unsigned long arg)
>  			sg_req_info_t *rinfo;
>  			unsigned int ms;
>  
> -			rinfo = kmalloc(SZ_SG_REQ_INFO * SG_MAX_QUEUE,
> -								GFP_KERNEL);
> +			rinfo = kzalloc(SZ_SG_REQ_INFO * SG_MAX_QUEUE,
> +					GFP_KERNEL);
>  			if (!rinfo)
>  				return -ENOMEM;
>  			read_lock_irqsave(&sfp->rq_list_lock, iflags);
>  			for (srp = sfp->headrp, val = 0; val < SG_MAX_QUEUE;
>  			     ++val, srp = srp ? srp->nextrp : srp) {
> -				memset(&rinfo[val], 0, SZ_SG_REQ_INFO);
>  				if (srp) {
>  					rinfo[val].req_state = srp->done + 1;
>  					rinfo[val].problem =
> 
Looks sane.

Acked-by: Colin Ian King <colin.king@canonical.com>
Stefan Bader July 19, 2018, 12:20 p.m. UTC | #2 
On 11.07.2018 17:45, Kleber Sacilotto de Souza wrote:
> From: Hannes Reinecke <hare@suse.de>
> 
> When calling SG_GET_REQUEST_TABLE ioctl only a half-filled table is
> returned; the remaining part will then contain stale kernel memory
> information.  This patch zeroes out the entire table to avoid this
> issue.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.com>
> Reviewed-by: Bart Van Assche <bart.vanassche@wdc.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Eric Dumazet <edumazet@google.com>
> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
> 
> CVE-2017-14991
> (backported from commit 3e0097499839e0fe3af380410eababe5a47c4cf9)
> Signed-off-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com>
Acked-by: Stefan Bader <stefan.bader@canonical.com>
> ---
>  drivers/scsi/sg.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
> index 47b8f7b8b7b4..1c3dd355b317 100644
> --- a/drivers/scsi/sg.c
> +++ b/drivers/scsi/sg.c
> @@ -1003,14 +1003,13 @@ sg_ioctl(struct file *filp, unsigned int cmd_in, unsigned long arg)
>  			sg_req_info_t *rinfo;
>  			unsigned int ms;
>  
> -			rinfo = kmalloc(SZ_SG_REQ_INFO * SG_MAX_QUEUE,
> -								GFP_KERNEL);
> +			rinfo = kzalloc(SZ_SG_REQ_INFO * SG_MAX_QUEUE,
> +					GFP_KERNEL);
>  			if (!rinfo)
>  				return -ENOMEM;
>  			read_lock_irqsave(&sfp->rq_list_lock, iflags);
>  			for (srp = sfp->headrp, val = 0; val < SG_MAX_QUEUE;
>  			     ++val, srp = srp ? srp->nextrp : srp) {
> -				memset(&rinfo[val], 0, SZ_SG_REQ_INFO);
>  				if (srp) {
>  					rinfo[val].req_state = srp->done + 1;
>  					rinfo[val].problem =
>
Juerg Haefliger July 23, 2018, 12:12 p.m. UTC | #3 
Applied to Trusty master-next.

...Juerg

On 07/11/2018 05:45 PM, Kleber Sacilotto de Souza wrote:
> From: Hannes Reinecke <hare@suse.de>
> 
> When calling SG_GET_REQUEST_TABLE ioctl only a half-filled table is
> returned; the remaining part will then contain stale kernel memory
> information.  This patch zeroes out the entire table to avoid this
> issue.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.com>
> Reviewed-by: Bart Van Assche <bart.vanassche@wdc.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Eric Dumazet <edumazet@google.com>
> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
> 
> CVE-2017-14991
> (backported from commit 3e0097499839e0fe3af380410eababe5a47c4cf9)
> Signed-off-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com>
> ---
>  drivers/scsi/sg.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
> index 47b8f7b8b7b4..1c3dd355b317 100644
> --- a/drivers/scsi/sg.c
> +++ b/drivers/scsi/sg.c
> @@ -1003,14 +1003,13 @@ sg_ioctl(struct file *filp, unsigned int cmd_in, unsigned long arg)
>  			sg_req_info_t *rinfo;
>  			unsigned int ms;
>  
> -			rinfo = kmalloc(SZ_SG_REQ_INFO * SG_MAX_QUEUE,
> -								GFP_KERNEL);
> +			rinfo = kzalloc(SZ_SG_REQ_INFO * SG_MAX_QUEUE,
> +					GFP_KERNEL);
>  			if (!rinfo)
>  				return -ENOMEM;
>  			read_lock_irqsave(&sfp->rq_list_lock, iflags);
>  			for (srp = sfp->headrp, val = 0; val < SG_MAX_QUEUE;
>  			     ++val, srp = srp ? srp->nextrp : srp) {
> -				memset(&rinfo[val], 0, SZ_SG_REQ_INFO);
>  				if (srp) {
>  					rinfo[val].req_state = srp->done + 1;
>  					rinfo[val].problem =
>
diff mbox series

Patch

diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 47b8f7b8b7b4..1c3dd355b317 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -1003,14 +1003,13 @@  sg_ioctl(struct file *filp, unsigned int cmd_in, unsigned long arg)
 			sg_req_info_t *rinfo;
 			unsigned int ms;
 
-			rinfo = kmalloc(SZ_SG_REQ_INFO * SG_MAX_QUEUE,
-								GFP_KERNEL);
+			rinfo = kzalloc(SZ_SG_REQ_INFO * SG_MAX_QUEUE,
+					GFP_KERNEL);
 			if (!rinfo)
 				return -ENOMEM;
 			read_lock_irqsave(&sfp->rq_list_lock, iflags);
 			for (srp = sfp->headrp, val = 0; val < SG_MAX_QUEUE;
 			     ++val, srp = srp ? srp->nextrp : srp) {
-				memset(&rinfo[val], 0, SZ_SG_REQ_INFO);
 				if (srp) {
 					rinfo[val].req_state = srp->done + 1;
 					rinfo[val].problem =