All of lore.kernel.org
 help / color / mirror / Atom feed
From: walter harms <wharms@bfs.de>
To: Colin King <colin.king@canonical.com>
Cc: Raghava Aditya Renukunta <RaghavaAditya.Renukunta@microsemi.com>,
	Adaptec OEM Raid Solutions <aacraid@microsemi.com>,
	"James E . J . Bottomley" <jejb@linux.vnet.ibm.com>,
	"Martin K . Petersen" <martin.petersen@oracle.com>,
	linux-scsi@vger.kernel.org, kernel-janitors@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] scsi: aacraid: fix leak of data from stack back to userspace
Date: Mon, 15 May 2017 15:07:10 +0000	[thread overview]
Message-ID: <5919C41E.7090309@bfs.de> (raw)
In-Reply-To: <20170515145605.21461-1-colin.king@canonical.com>



Am 15.05.2017 16:56, schrieb Colin King:
> From: Colin Ian King <colin.king@canonical.com>
> 
> The fields sense_data_size and sense_data are unitialized garbage from
> the stack and are being copied back to userspace.  Fix this leak of
> stack information by ensuring they are zero'd.
> 
> Detected by CoverityScan, CID#1435473 ("Uninitialized scalar variable")
> 
> Fixes: 423400e64d377 ("scsi: aacraid: Include HBA direct interface")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>  drivers/scsi/aacraid/commctrl.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/scsi/aacraid/commctrl.c b/drivers/scsi/aacraid/commctrl.c
> index 106b9332f718..6bb6ed48e31d 100644
> --- a/drivers/scsi/aacraid/commctrl.c
> +++ b/drivers/scsi/aacraid/commctrl.c
> @@ -955,6 +955,8 @@ static int aac_send_raw_srb(struct aac_dev* dev, void __user * arg)
>  			reply.srb_status = SRB_STATUS_SUCCESS;
>  			reply.scsi_status = 0;
>  			reply.data_xfer_length = byte_count;
> +			reply.sense_data_size = 0;
> +			memset(reply.sense_data, 0, AAC_SENSE_BUFFERSIZE);
>  		} else {
>  			reply.srb_status = err->service_response;
>  			reply.scsi_status = err->status;


an other idea would be initialize the reply with zero:
struct aac_srb_reply reply={0};

maybe that is more future proof but i have no idea about the speed penalty.

just my 2 cents,
 wh

WARNING: multiple messages have this Message-ID (diff)
From: walter harms <wharms@bfs.de>
To: Colin King <colin.king@canonical.com>
Cc: Raghava Aditya Renukunta <RaghavaAditya.Renukunta@microsemi.com>,
	Adaptec OEM Raid Solutions <aacraid@microsemi.com>,
	"James E . J . Bottomley" <jejb@linux.vnet.ibm.com>,
	"Martin K . Petersen" <martin.petersen@oracle.com>,
	linux-scsi@vger.kernel.org, kernel-janitors@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] scsi: aacraid: fix leak of data from stack back to userspace
Date: Mon, 15 May 2017 17:07:10 +0200	[thread overview]
Message-ID: <5919C41E.7090309@bfs.de> (raw)
In-Reply-To: <20170515145605.21461-1-colin.king@canonical.com>



Am 15.05.2017 16:56, schrieb Colin King:
> From: Colin Ian King <colin.king@canonical.com>
> 
> The fields sense_data_size and sense_data are unitialized garbage from
> the stack and are being copied back to userspace.  Fix this leak of
> stack information by ensuring they are zero'd.
> 
> Detected by CoverityScan, CID#1435473 ("Uninitialized scalar variable")
> 
> Fixes: 423400e64d377 ("scsi: aacraid: Include HBA direct interface")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>  drivers/scsi/aacraid/commctrl.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/scsi/aacraid/commctrl.c b/drivers/scsi/aacraid/commctrl.c
> index 106b9332f718..6bb6ed48e31d 100644
> --- a/drivers/scsi/aacraid/commctrl.c
> +++ b/drivers/scsi/aacraid/commctrl.c
> @@ -955,6 +955,8 @@ static int aac_send_raw_srb(struct aac_dev* dev, void __user * arg)
>  			reply.srb_status = SRB_STATUS_SUCCESS;
>  			reply.scsi_status = 0;
>  			reply.data_xfer_length = byte_count;
> +			reply.sense_data_size = 0;
> +			memset(reply.sense_data, 0, AAC_SENSE_BUFFERSIZE);
>  		} else {
>  			reply.srb_status = err->service_response;
>  			reply.scsi_status = err->status;


an other idea would be initialize the reply with zero:
struct aac_srb_reply reply={0};

maybe that is more future proof but i have no idea about the speed penalty.

just my 2 cents,
 wh

  reply	other threads:[~2017-05-15 15:07 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-15 14:56 [PATCH] scsi: aacraid: fix leak of data from stack back to userspace Colin King
2017-05-15 14:56 ` Colin King
2017-05-15 15:07 ` walter harms [this message]
2017-05-15 15:07   ` walter harms
2017-06-13 21:06 ` Dave Carroll
2017-06-13 21:06   ` Dave Carroll

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5919C41E.7090309@bfs.de \
    --to=wharms@bfs.de \
    --cc=RaghavaAditya.Renukunta@microsemi.com \
    --cc=aacraid@microsemi.com \
    --cc=colin.king@canonical.com \
    --cc=jejb@linux.vnet.ibm.com \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.