| Message ID | 1503581525-21462-1-git-send-email-stefan.bader@canonical.com |
|---|---|
| State | New |
| Headers | show |
On 24/08/17 14:32, Stefan Bader wrote: > From bf33f87dd04c371ea33feb821b60d63d754e3124 Mon Sep 17 00:00:00 2001 > From: peter chang <dpf@google.com> > Date: Wed, 15 Feb 2017 14:11:54 -0800 > Subject: [PATCH] scsi: sg: check length passed to SG_NEXT_CMD_LEN > > The user can control the size of the next command passed along, but the > value passed to the ioctl isn't checked against the usable max command > size. > > Cc: <stable@vger.kernel.org> > Signed-off-by: Peter Chang <dpf@google.com> > Acked-by: Douglas Gilbert <dgilbert@interlog.com> > Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com> > > CVE-2017-7187 > > (backported from commit bf33f87dd04c371ea33feb821b60d63d754e3124) > [smb: SG_MAX_CDB_SIZE -> MAX_COMMAND_SIZE] > Signed-off-by: Stefan Bader <stefan.bader@canonical.com> > --- > Notes: > - Strictly speaking this is _not_ needed in Trusty as back then there > was a size check in sg_write which was removed when introducing > SG_MAX_CDB_SIZE: > 65c26a0 sg: relax 16 byte cdb restriction > - Backporting the commit anyway would have the advantage of returning > the error sooner (when trying to set the next command size). > - So to resolve the CVE for Trusty we could either update the breaks- > fix entry or apply the backport. > > -Stefan > > drivers/scsi/sg.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c > index e831e01..849ff810 100644 > --- a/drivers/scsi/sg.c > +++ b/drivers/scsi/sg.c > @@ -996,6 +996,8 @@ sg_ioctl(struct file *filp, unsigned int cmd_in, unsigned long arg) > result = get_user(val, ip); > if (result) > return result; > + if (val > MAX_COMMAND_SIZE) > + return -ENOMEM; > sfp->next_cmd_len = (val > 0) ? val : 0; > return 0; > case SG_GET_VERSION_NUM: > From Stefan's comments about the original 16 byte command limitation I concur with his analysis for the backport. Looks good to me. Acked-by: Colin Ian King <colin.king@canonical.com>
On 08/24/17 15:32, Stefan Bader wrote: > From bf33f87dd04c371ea33feb821b60d63d754e3124 Mon Sep 17 00:00:00 2001 > From: peter chang <dpf@google.com> > Date: Wed, 15 Feb 2017 14:11:54 -0800 > Subject: [PATCH] scsi: sg: check length passed to SG_NEXT_CMD_LEN > > The user can control the size of the next command passed along, but the > value passed to the ioctl isn't checked against the usable max command > size. > > Cc: <stable@vger.kernel.org> > Signed-off-by: Peter Chang <dpf@google.com> > Acked-by: Douglas Gilbert <dgilbert@interlog.com> > Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com> > > CVE-2017-7187 > > (backported from commit bf33f87dd04c371ea33feb821b60d63d754e3124) > [smb: SG_MAX_CDB_SIZE -> MAX_COMMAND_SIZE] > Signed-off-by: Stefan Bader <stefan.bader@canonical.com> Acked-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com> > --- > Notes: > - Strictly speaking this is _not_ needed in Trusty as back then there > was a size check in sg_write which was removed when introducing > SG_MAX_CDB_SIZE: > 65c26a0 sg: relax 16 byte cdb restriction > - Backporting the commit anyway would have the advantage of returning > the error sooner (when trying to set the next command size). > - So to resolve the CVE for Trusty we could either update the breaks- > fix entry or apply the backport. > > -Stefan > > drivers/scsi/sg.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c > index e831e01..849ff810 100644 > --- a/drivers/scsi/sg.c > +++ b/drivers/scsi/sg.c > @@ -996,6 +996,8 @@ sg_ioctl(struct file *filp, unsigned int cmd_in, unsigned long arg) > result = get_user(val, ip); > if (result) > return result; > + if (val > MAX_COMMAND_SIZE) > + return -ENOMEM; > sfp->next_cmd_len = (val > 0) ? val : 0; > return 0; > case SG_GET_VERSION_NUM: >
Applied to trusty/master-next branch. Thanks.
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c index e831e01..849ff810 100644 --- a/drivers/scsi/sg.c +++ b/drivers/scsi/sg.c @@ -996,6 +996,8 @@ sg_ioctl(struct file *filp, unsigned int cmd_in, unsigned long arg) result = get_user(val, ip); if (result) return result; + if (val > MAX_COMMAND_SIZE) + return -ENOMEM; sfp->next_cmd_len = (val > 0) ? val : 0; return 0; case SG_GET_VERSION_NUM: