2022-08-18 22:07:13

by Wolfram Sang

[permalink] [raw]
Subject: [PATCH] s390: move from strlcpy with unused retval to strscpy

Follow the advice of the below link and prefer 'strscpy' in this
subsystem. Conversion is 1:1 because the return value is not used.
Generated by a coccinelle script.

Link: https://lore.kernel.org/r/CAHk-=wgfRnXz0W3D37d01q3JFkr_i_uTL=V6A6G1oUZcprmknw@mail.gmail.com/
Signed-off-by: Wolfram Sang <[email protected]>
---
drivers/s390/block/dasd_devmap.c | 2 +-
drivers/s390/block/dasd_eer.c | 4 ++--
drivers/s390/block/dcssblk.c | 2 +-
drivers/s390/char/hmcdrv_cache.c | 2 +-
drivers/s390/char/tape_class.c | 4 ++--
drivers/s390/cio/qdio_debug.c | 2 +-
drivers/s390/net/ctcm_main.c | 2 +-
drivers/s390/net/fsm.c | 2 +-
drivers/s390/net/qeth_ethtool.c | 4 ++--
drivers/s390/scsi/zfcp_aux.c | 2 +-
drivers/s390/scsi/zfcp_fc.c | 2 +-
11 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/s390/block/dasd_devmap.c b/drivers/s390/block/dasd_devmap.c
index 811e79c9f59c..0062a70f0128 100644
--- a/drivers/s390/block/dasd_devmap.c
+++ b/drivers/s390/block/dasd_devmap.c
@@ -426,7 +426,7 @@ dasd_add_busid(const char *bus_id, int features)
if (!devmap) {
/* This bus_id is new. */
new->devindex = dasd_max_devindex++;
- strlcpy(new->bus_id, bus_id, DASD_BUS_ID_SIZE);
+ strscpy(new->bus_id, bus_id, DASD_BUS_ID_SIZE);
new->features = features;
new->device = NULL;
list_add(&new->list, &dasd_hashlists[hash]);
diff --git a/drivers/s390/block/dasd_eer.c b/drivers/s390/block/dasd_eer.c
index 5ae64af9ccea..d4d31cd11d26 100644
--- a/drivers/s390/block/dasd_eer.c
+++ b/drivers/s390/block/dasd_eer.c
@@ -313,7 +313,7 @@ static void dasd_eer_write_standard_trigger(struct dasd_device *device,
ktime_get_real_ts64(&ts);
header.tv_sec = ts.tv_sec;
header.tv_usec = ts.tv_nsec / NSEC_PER_USEC;
- strlcpy(header.busid, dev_name(&device->cdev->dev),
+ strscpy(header.busid, dev_name(&device->cdev->dev),
DASD_EER_BUSID_SIZE);

spin_lock_irqsave(&bufferlock, flags);
@@ -356,7 +356,7 @@ static void dasd_eer_write_snss_trigger(struct dasd_device *device,
ktime_get_real_ts64(&ts);
header.tv_sec = ts.tv_sec;
header.tv_usec = ts.tv_nsec / NSEC_PER_USEC;
- strlcpy(header.busid, dev_name(&device->cdev->dev),
+ strscpy(header.busid, dev_name(&device->cdev->dev),
DASD_EER_BUSID_SIZE);

spin_lock_irqsave(&bufferlock, flags);
diff --git a/drivers/s390/block/dcssblk.c b/drivers/s390/block/dcssblk.c
index 5187705bd0f3..93b80da60277 100644
--- a/drivers/s390/block/dcssblk.c
+++ b/drivers/s390/block/dcssblk.c
@@ -614,7 +614,7 @@ dcssblk_add_store(struct device *dev, struct device_attribute *attr, const char
rc = -ENAMETOOLONG;
goto seg_list_del;
}
- strlcpy(local_buf, buf, i + 1);
+ strscpy(local_buf, buf, i + 1);
dev_info->num_of_segments = num_of_segments;
rc = dcssblk_is_continuous(dev_info);
if (rc < 0)
diff --git a/drivers/s390/char/hmcdrv_cache.c b/drivers/s390/char/hmcdrv_cache.c
index 1f5bdb237862..43df27ceec11 100644
--- a/drivers/s390/char/hmcdrv_cache.c
+++ b/drivers/s390/char/hmcdrv_cache.c
@@ -154,7 +154,7 @@ static ssize_t hmcdrv_cache_do(const struct hmcdrv_ftp_cmdspec *ftp,
/* cache some file info (FTP command, file name and file
* size) unconditionally
*/
- strlcpy(hmcdrv_cache_file.fname, ftp->fname,
+ strscpy(hmcdrv_cache_file.fname, ftp->fname,
HMCDRV_FTP_FIDENT_MAX);
hmcdrv_cache_file.id = ftp->id;
pr_debug("caching cmd %d, file size %zu for '%s'\n",
diff --git a/drivers/s390/char/tape_class.c b/drivers/s390/char/tape_class.c
index b58df0dd0039..c21dc68e05a0 100644
--- a/drivers/s390/char/tape_class.c
+++ b/drivers/s390/char/tape_class.c
@@ -54,10 +54,10 @@ struct tape_class_device *register_tape_dev(
if (!tcd)
return ERR_PTR(-ENOMEM);

- strlcpy(tcd->device_name, device_name, TAPECLASS_NAME_LEN);
+ strscpy(tcd->device_name, device_name, TAPECLASS_NAME_LEN);
for (s = strchr(tcd->device_name, '/'); s; s = strchr(s, '/'))
*s = '!';
- strlcpy(tcd->mode_name, mode_name, TAPECLASS_NAME_LEN);
+ strscpy(tcd->mode_name, mode_name, TAPECLASS_NAME_LEN);
for (s = strchr(tcd->mode_name, '/'); s; s = strchr(s, '/'))
*s = '!';

diff --git a/drivers/s390/cio/qdio_debug.c b/drivers/s390/cio/qdio_debug.c
index 4bb7965daa0f..1a9714af51e4 100644
--- a/drivers/s390/cio/qdio_debug.c
+++ b/drivers/s390/cio/qdio_debug.c
@@ -87,7 +87,7 @@ int qdio_allocate_dbf(struct qdio_irq *irq_ptr)
debug_unregister(irq_ptr->debug_area);
return -ENOMEM;
}
- strlcpy(new_entry->dbf_name, text, QDIO_DBF_NAME_LEN);
+ strscpy(new_entry->dbf_name, text, QDIO_DBF_NAME_LEN);
new_entry->dbf_info = irq_ptr->debug_area;
mutex_lock(&qdio_dbf_list_mutex);
list_add(&new_entry->dbf_list, &qdio_dbf_list);
diff --git a/drivers/s390/net/ctcm_main.c b/drivers/s390/net/ctcm_main.c
index e0fdd54bfeb7..37b551bd43bf 100644
--- a/drivers/s390/net/ctcm_main.c
+++ b/drivers/s390/net/ctcm_main.c
@@ -1566,7 +1566,7 @@ static int ctcm_new_device(struct ccwgroup_device *cgdev)
goto out_dev;
}

- strlcpy(priv->fsm->name, dev->name, sizeof(priv->fsm->name));
+ strscpy(priv->fsm->name, dev->name, sizeof(priv->fsm->name));

dev_info(&dev->dev,
"setup OK : r/w = %s/%s, protocol : %d\n",
diff --git a/drivers/s390/net/fsm.c b/drivers/s390/net/fsm.c
index 98c4864932d2..0ff61d00feb1 100644
--- a/drivers/s390/net/fsm.c
+++ b/drivers/s390/net/fsm.c
@@ -28,7 +28,7 @@ init_fsm(char *name, const char **state_names, const char **event_names, int nr_
"fsm(%s): init_fsm: Couldn't alloc instance\n", name);
return NULL;
}
- strlcpy(this->name, name, sizeof(this->name));
+ strscpy(this->name, name, sizeof(this->name));
init_waitqueue_head(&this->wait_q);

f = kzalloc(sizeof(fsm), order);
diff --git a/drivers/s390/net/qeth_ethtool.c b/drivers/s390/net/qeth_ethtool.c
index 9eba0a32e9f9..e250f49535fa 100644
--- a/drivers/s390/net/qeth_ethtool.c
+++ b/drivers/s390/net/qeth_ethtool.c
@@ -188,9 +188,9 @@ static void qeth_get_drvinfo(struct net_device *dev,
{
struct qeth_card *card = dev->ml_priv;

- strlcpy(info->driver, IS_LAYER2(card) ? "qeth_l2" : "qeth_l3",
+ strscpy(info->driver, IS_LAYER2(card) ? "qeth_l2" : "qeth_l3",
sizeof(info->driver));
- strlcpy(info->fw_version, card->info.mcl_level,
+ strscpy(info->fw_version, card->info.mcl_level,
sizeof(info->fw_version));
snprintf(info->bus_info, sizeof(info->bus_info), "%s/%s/%s",
CARD_RDEV_ID(card), CARD_WDEV_ID(card), CARD_DDEV_ID(card));
diff --git a/drivers/s390/scsi/zfcp_aux.c b/drivers/s390/scsi/zfcp_aux.c
index fd2f1c31bd21..df782646e856 100644
--- a/drivers/s390/scsi/zfcp_aux.c
+++ b/drivers/s390/scsi/zfcp_aux.c
@@ -103,7 +103,7 @@ static void __init zfcp_init_device_setup(char *devstr)
token = strsep(&str, ",");
if (!token || strlen(token) >= ZFCP_BUS_ID_SIZE)
goto err_out;
- strlcpy(busid, token, ZFCP_BUS_ID_SIZE);
+ strscpy(busid, token, ZFCP_BUS_ID_SIZE);

token = strsep(&str, ",");
if (!token || kstrtoull(token, 0, (unsigned long long *) &wwpn))
diff --git a/drivers/s390/scsi/zfcp_fc.c b/drivers/s390/scsi/zfcp_fc.c
index b61acbb09be3..77917b339870 100644
--- a/drivers/s390/scsi/zfcp_fc.c
+++ b/drivers/s390/scsi/zfcp_fc.c
@@ -885,7 +885,7 @@ static int zfcp_fc_gspn(struct zfcp_adapter *adapter,
dev_name(&adapter->ccw_device->dev),
init_utsname()->nodename);
else
- strlcpy(fc_host_symbolic_name(adapter->scsi_host),
+ strscpy(fc_host_symbolic_name(adapter->scsi_host),
gspn_rsp->gspn.fp_name, FC_SYMBOLIC_NAME_SIZE);

return 0;
--
2.35.1


2022-08-19 08:17:04

by Alexandra Winter

[permalink] [raw]
Subject: Re: [PATCH] s390: move from strlcpy with unused retval to strscpy



On 18.08.22 23:01, Wolfram Sang wrote:
> Follow the advice of the below link and prefer 'strscpy' in this
> subsystem. Conversion is 1:1 because the return value is not used.
> Generated by a coccinelle script.
>
> Link: https://lore.kernel.org/r/CAHk-=wgfRnXz0W3D37d01q3JFkr_i_uTL=V6A6G1oUZcprmknw@mail.gmail.com/
> Signed-off-by: Wolfram Sang <[email protected]>
> ---
> drivers/s390/block/dasd_devmap.c | 2 +-
> drivers/s390/block/dasd_eer.c | 4 ++--
> drivers/s390/block/dcssblk.c | 2 +-
> drivers/s390/char/hmcdrv_cache.c | 2 +-
> drivers/s390/char/tape_class.c | 4 ++--
> drivers/s390/cio/qdio_debug.c | 2 +-
> drivers/s390/net/ctcm_main.c | 2 +-
> drivers/s390/net/fsm.c | 2 +-
> drivers/s390/net/qeth_ethtool.c | 4 ++--
> drivers/s390/scsi/zfcp_aux.c | 2 +-
> drivers/s390/scsi/zfcp_fc.c | 2 +-
> 11 files changed, 14 insertions(+), 14 deletions(-)
>


Thank you. Ack'ed for drivers/s390/net and drivers/s390/cio
Acked-by: Alexandra Winter <[email protected]>

2022-08-19 10:47:55

by Benjamin Block

[permalink] [raw]
Subject: Re: [PATCH] s390: move from strlcpy with unused retval to strscpy

On Thu, Aug 18, 2022 at 11:01:01PM +0200, Wolfram Sang wrote:
> Follow the advice of the below link and prefer 'strscpy' in this
> subsystem. Conversion is 1:1 because the return value is not used.
> Generated by a coccinelle script.
>
> Link: https://lore.kernel.org/r/CAHk-=wgfRnXz0W3D37d01q3JFkr_i_uTL=V6A6G1oUZcprmknw@mail.gmail.com/
> Signed-off-by: Wolfram Sang <[email protected]>
> ---
> drivers/s390/block/dasd_devmap.c | 2 +-
> drivers/s390/block/dasd_eer.c | 4 ++--
> drivers/s390/block/dcssblk.c | 2 +-
> drivers/s390/char/hmcdrv_cache.c | 2 +-
> drivers/s390/char/tape_class.c | 4 ++--
> drivers/s390/cio/qdio_debug.c | 2 +-
> drivers/s390/net/ctcm_main.c | 2 +-
> drivers/s390/net/fsm.c | 2 +-
> drivers/s390/net/qeth_ethtool.c | 4 ++--
> drivers/s390/scsi/zfcp_aux.c | 2 +-
> drivers/s390/scsi/zfcp_fc.c | 2 +-
> 11 files changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/s390/cio/qdio_debug.c b/drivers/s390/cio/qdio_debug.c
> index 4bb7965daa0f..1a9714af51e4 100644
> --- a/drivers/s390/cio/qdio_debug.c
> +++ b/drivers/s390/cio/qdio_debug.c
> @@ -87,7 +87,7 @@ int qdio_allocate_dbf(struct qdio_irq *irq_ptr)
> debug_unregister(irq_ptr->debug_area);
> return -ENOMEM;
> }
> - strlcpy(new_entry->dbf_name, text, QDIO_DBF_NAME_LEN);
> + strscpy(new_entry->dbf_name, text, QDIO_DBF_NAME_LEN);
> new_entry->dbf_info = irq_ptr->debug_area;
> mutex_lock(&qdio_dbf_list_mutex);
> list_add(&new_entry->dbf_list, &qdio_dbf_list);
> diff --git a/drivers/s390/scsi/zfcp_aux.c b/drivers/s390/scsi/zfcp_aux.c
> index fd2f1c31bd21..df782646e856 100644
> --- a/drivers/s390/scsi/zfcp_aux.c
> +++ b/drivers/s390/scsi/zfcp_aux.c
> @@ -103,7 +103,7 @@ static void __init zfcp_init_device_setup(char *devstr)
> token = strsep(&str, ",");
> if (!token || strlen(token) >= ZFCP_BUS_ID_SIZE)
> goto err_out;
> - strlcpy(busid, token, ZFCP_BUS_ID_SIZE);
> + strscpy(busid, token, ZFCP_BUS_ID_SIZE);
>
> token = strsep(&str, ",");
> if (!token || kstrtoull(token, 0, (unsigned long long *) &wwpn))
> diff --git a/drivers/s390/scsi/zfcp_fc.c b/drivers/s390/scsi/zfcp_fc.c
> index b61acbb09be3..77917b339870 100644
> --- a/drivers/s390/scsi/zfcp_fc.c
> +++ b/drivers/s390/scsi/zfcp_fc.c
> @@ -885,7 +885,7 @@ static int zfcp_fc_gspn(struct zfcp_adapter *adapter,
> dev_name(&adapter->ccw_device->dev),
> init_utsname()->nodename);
> else
> - strlcpy(fc_host_symbolic_name(adapter->scsi_host),
> + strscpy(fc_host_symbolic_name(adapter->scsi_host),
> gspn_rsp->gspn.fp_name, FC_SYMBOLIC_NAME_SIZE);
>
> return 0;
> --
> 2.35.1
>

Those look good to me.

As far as zFCP and QDIO go:

Acked-by: Benjamin Block <[email protected]>

--
Best Regards, Benjamin Block / Linux on IBM Z Kernel Development / IBM Systems
IBM Deutschland Research & Development GmbH / https://www.ibm.com/privacy
Vorsitz. AufsR.: Gregor Pillen / Gesch?ftsf?hrung: David Faller
Sitz der Gesellschaft: B?blingen / Registergericht: AmtsG Stuttgart, HRB 243294

2022-08-22 08:45:10

by Alexander Gordeev

[permalink] [raw]
Subject: Re: [PATCH] s390: move from strlcpy with unused retval to strscpy

On Thu, Aug 18, 2022 at 11:01:01PM +0200, Wolfram Sang wrote:
Hi Wolfram,
> Follow the advice of the below link and prefer 'strscpy' in this
> subsystem. Conversion is 1:1 because the return value is not used.
> Generated by a coccinelle script.

Could you please explain why you skipped strlcpy() usage in
drivers/s390/char/diag_ftp.c and drivers/s390/char/sclp_ftp.c?

Thanks!

2022-08-22 09:04:14

by Wolfram Sang

[permalink] [raw]
Subject: Re: [PATCH] s390: move from strlcpy with unused retval to strscpy

Hi Alexander,

> Could you please explain why you skipped strlcpy() usage in
> drivers/s390/char/diag_ftp.c and drivers/s390/char/sclp_ftp.c?

Sure. It is a bit hidden in $subject, but the key is to convert strlcpy
instances for now which do not check the return value. This is the
low-hanging fruit.

Converting the other uses checking the return value needs to be done
manually and much more carfully. I wanted to this as a second step, but
if you prefer to have everything converted in one go, I can give your
subsystem a priority boost. Would you prefer that?

Thanks and happy hacking,

Wolfram


Attachments:
(No filename) (622.00 B)
signature.asc (849.00 B)
Download all attachments

2022-08-22 11:54:09

by Alexander Gordeev

[permalink] [raw]
Subject: Re: [PATCH] s390: move from strlcpy with unused retval to strscpy

On Mon, Aug 22, 2022 at 10:37:47AM +0200, Wolfram Sang wrote:
> Hi Alexander,
>
> > Could you please explain why you skipped strlcpy() usage in
> > drivers/s390/char/diag_ftp.c and drivers/s390/char/sclp_ftp.c?
>
> Sure. It is a bit hidden in $subject, but the key is to convert strlcpy
> instances for now which do not check the return value. This is the
> low-hanging fruit.
>
> Converting the other uses checking the return value needs to be done
> manually and much more carfully. I wanted to this as a second step, but
> if you prefer to have everything converted in one go, I can give your
> subsystem a priority boost. Would you prefer that?

A follow-up patch is also fine.

I guess, you also wanted a fix for arch/s390/kvm/tests/instr_icpt/main.c
in this series.

> Thanks and happy hacking,
>
> Wolfram

Thanks!

2022-08-23 01:05:39

by Jakub Kicinski

[permalink] [raw]
Subject: Re: [PATCH] s390: move from strlcpy with unused retval to strscpy

On Thu, 18 Aug 2022 23:01:01 +0200 Wolfram Sang wrote:
> Follow the advice of the below link and prefer 'strscpy' in this
> subsystem. Conversion is 1:1 because the return value is not used.
> Generated by a coccinelle script.
>
> Link: https://lore.kernel.org/r/CAHk-=wgfRnXz0W3D37d01q3JFkr_i_uTL=V6A6G1oUZcprmknw@mail.gmail.com/
> Signed-off-by: Wolfram Sang <[email protected]>
> ---
> drivers/s390/block/dasd_devmap.c | 2 +-
> drivers/s390/block/dasd_eer.c | 4 ++--
> drivers/s390/block/dcssblk.c | 2 +-
> drivers/s390/char/hmcdrv_cache.c | 2 +-
> drivers/s390/char/tape_class.c | 4 ++--
> drivers/s390/cio/qdio_debug.c | 2 +-
> drivers/s390/net/ctcm_main.c | 2 +-
> drivers/s390/net/fsm.c | 2 +-
> drivers/s390/net/qeth_ethtool.c | 4 ++--
> drivers/s390/scsi/zfcp_aux.c | 2 +-
> drivers/s390/scsi/zfcp_fc.c | 2 +-

I'm assuming this will go via the s390 tree?

Acked-by: Jakub Kicinski <[email protected]>

If nobody picks it up please feel free to resend the networking parts to us.

2022-08-23 13:12:45

by Alexander Gordeev

[permalink] [raw]
Subject: Re: [PATCH] s390: move from strlcpy with unused retval to strscpy

On Mon, Aug 22, 2022 at 01:48:54PM +0200, Alexander Gordeev wrote:
> I guess, you also wanted a fix for arch/s390/kvm/tests/instr_icpt/main.c
> in this series.

Please, ignore this one.

Thanks!

2022-08-23 15:28:05

by Vasily Gorbik

[permalink] [raw]
Subject: Re: [PATCH] s390: move from strlcpy with unused retval to strscpy

On Thu, Aug 18, 2022 at 11:01:01PM +0200, Wolfram Sang wrote:
> Follow the advice of the below link and prefer 'strscpy' in this
> subsystem. Conversion is 1:1 because the return value is not used.
> Generated by a coccinelle script.
>
> Link: https://lore.kernel.org/r/CAHk-=wgfRnXz0W3D37d01q3JFkr_i_uTL=V6A6G1oUZcprmknw@mail.gmail.com/
> Signed-off-by: Wolfram Sang <[email protected]>

Squashed it together with
https://lore.kernel.org/r/[email protected]
and applied to s390 tree. Thank you!

2022-08-23 16:52:08

by Vasily Gorbik

[permalink] [raw]
Subject: Re: [PATCH] s390: move from strlcpy with unused retval to strscpy

On Mon, Aug 22, 2022 at 06:02:49PM -0700, Jakub Kicinski wrote:
> On Thu, 18 Aug 2022 23:01:01 +0200 Wolfram Sang wrote:
> > Follow the advice of the below link and prefer 'strscpy' in this
> > subsystem. Conversion is 1:1 because the return value is not used.
> > Generated by a coccinelle script.
> >
> > Link: https://lore.kernel.org/r/CAHk-=wgfRnXz0W3D37d01q3JFkr_i_uTL=V6A6G1oUZcprmknw@mail.gmail.com/
> > Signed-off-by: Wolfram Sang <[email protected]>
> > ---
> > drivers/s390/block/dasd_devmap.c | 2 +-
> > drivers/s390/block/dasd_eer.c | 4 ++--
> > drivers/s390/block/dcssblk.c | 2 +-
> > drivers/s390/char/hmcdrv_cache.c | 2 +-
> > drivers/s390/char/tape_class.c | 4 ++--
> > drivers/s390/cio/qdio_debug.c | 2 +-
> > drivers/s390/net/ctcm_main.c | 2 +-
> > drivers/s390/net/fsm.c | 2 +-
> > drivers/s390/net/qeth_ethtool.c | 4 ++--
> > drivers/s390/scsi/zfcp_aux.c | 2 +-
> > drivers/s390/scsi/zfcp_fc.c | 2 +-
>
> I'm assuming this will go via the s390 tree?

Yes, I'll just take it via s390 tree. Thanks

>
> Acked-by: Jakub Kicinski <[email protected]>
>
> If nobody picks it up please feel free to resend the networking parts to us.