Hi,
This is (hopefully) the last version of the string fixups found in the
ibmvscsi driver.
Laura Abbott (2):
scsi: ibmvscsis: Fix a stringop-overflow warning
scsi: ibmvscsis: Ensure partition name is properly NUL terminated
drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
--
2.17.1
While reviewing another part of the code, Kees noticed that the
strncpy of the partition name might not always be NUL terminated. Switch
to using strscpy which does this safely.
Reported-by: Kees Cook <[email protected]>
Signed-off-by: Laura Abbott <[email protected]>
---
v2: Switch to strscpy instead of just strlcpy
---
drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
index b3a029ad07cd..f42a619198c4 100644
--- a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
+++ b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
@@ -3477,7 +3477,7 @@ static int ibmvscsis_probe(struct vio_dev *vdev,
snprintf(vscsi->eye, sizeof(vscsi->eye), "VSCSI %s", vdev->name);
vscsi->dds.unit_id = vdev->unit_address;
- strncpy(vscsi->dds.partition_name, partition_name,
+ strscpy(vscsi->dds.partition_name, partition_name,
sizeof(vscsi->dds.partition_name));
vscsi->dds.partition_num = partition_number;
--
2.17.1
There's currently a warning about string overflow with strncat:
drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c: In function 'ibmvscsis_probe':
drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c:3479:2: error: 'strncat' specified
bound 64 equals destination size [-Werror=stringop-overflow=]
strncat(vscsi->eye, vdev->name, MAX_EYE);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Switch to a single snprintf instead of a strcpy + strcat to handle this
cleanly.
Signed-off-by: Laura Abbott <[email protected]>
---
v3: Make sure there is an extra space in the partition name
---
drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
index fac377320158..b3a029ad07cd 100644
--- a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
+++ b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
@@ -3474,8 +3474,7 @@ static int ibmvscsis_probe(struct vio_dev *vdev,
vscsi->dds.window[LOCAL].liobn,
vscsi->dds.window[REMOTE].liobn);
- strcpy(vscsi->eye, "VSCSI ");
- strncat(vscsi->eye, vdev->name, MAX_EYE);
+ snprintf(vscsi->eye, sizeof(vscsi->eye), "VSCSI %s", vdev->name);
vscsi->dds.unit_id = vdev->unit_address;
strncpy(vscsi->dds.partition_name, partition_name,
--
2.17.1
On Tue, Sep 11, 2018 at 12:22 PM, Laura Abbott <[email protected]> wrote:
>
> While reviewing another part of the code, Kees noticed that the
> strncpy of the partition name might not always be NUL terminated. Switch
> to using strscpy which does this safely.
>
> Reported-by: Kees Cook <[email protected]>
> Signed-off-by: Laura Abbott <[email protected]>
Reviewed-by: Kees Cook <[email protected]>
-Kees
> ---
> v2: Switch to strscpy instead of just strlcpy
> ---
> drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
> index b3a029ad07cd..f42a619198c4 100644
> --- a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
> +++ b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
> @@ -3477,7 +3477,7 @@ static int ibmvscsis_probe(struct vio_dev *vdev,
> snprintf(vscsi->eye, sizeof(vscsi->eye), "VSCSI %s", vdev->name);
>
> vscsi->dds.unit_id = vdev->unit_address;
> - strncpy(vscsi->dds.partition_name, partition_name,
> + strscpy(vscsi->dds.partition_name, partition_name,
> sizeof(vscsi->dds.partition_name));
> vscsi->dds.partition_num = partition_number;
>
> --
> 2.17.1
>
--
Kees Cook
Pixel Security
Laura,
> This is (hopefully) the last version of the string fixups found in the
> ibmvscsi driver.
Applied to 4.19/scsi-fixes, thank you!
--
Martin K. Petersen Oracle Linux Engineering
> On Sep 11, 2018, at 2:22 PM, Laura Abbott <[email protected]> wrote:
>
> There's currently a warning about string overflow with strncat:
>
> drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c: In function 'ibmvscsis_probe':
> drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c:3479:2: error: 'strncat' specified
> bound 64 equals destination size [-Werror=stringop-overflow=]
> strncat(vscsi->eye, vdev->name, MAX_EYE);
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> Switch to a single snprintf instead of a strcpy + strcat to handle this
> cleanly.
>
> Signed-off-by: Laura Abbott <[email protected]>
> ---
> v3: Make sure there is an extra space in the partition name
> ---
> drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
Reviewed-by: Bryant G. Ly [email protected]
I sent a PR to update my email from [email protected] a week ago.
-Bryant
> On Sep 11, 2018, at 2:22 PM, Laura Abbott <[email protected]> wrote:
>
> While reviewing another part of the code, Kees noticed that the
> strncpy of the partition name might not always be NUL terminated. Switch
> to using strscpy which does this safely.
>
> Reported-by: Kees Cook <[email protected]>
> Signed-off-by: Laura Abbott <[email protected]>
> Reviewed-by: Kees Cook <[email protected]>
> ---
> v2: Switch to strscpy instead of just strlcpy
> ---
> drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
Reviewed-by: Bryant G. Ly [email protected]
I sent a PR to update my email from [email protected] a week ago.
-Bryant