| Message ID | 20250226185531.1092-2-thorsten.blum@linux.dev |
|---|---|
| State | Superseded |
| Headers | show |
| Series | scsi: elx: sli4: Replace deprecated strncpy() with strscpy() | expand |
On Wed, Feb 26, 2025 at 07:55:26PM +0100, Thorsten Blum wrote: > strncpy() is deprecated for NUL-terminated destination buffers; use > strscpy() instead. > > Compile-tested only. > > Link: https://github.com/KSPP/linux/issues/90 > Cc: linux-hardening@vger.kernel.org > Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev> > --- > drivers/scsi/elx/libefc_sli/sli4.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/scsi/elx/libefc_sli/sli4.c b/drivers/scsi/elx/libefc_sli/sli4.c > index 5e7fb110bc3f..d9a231fc0e0d 100644 > --- a/drivers/scsi/elx/libefc_sli/sli4.c > +++ b/drivers/scsi/elx/libefc_sli/sli4.c > @@ -3804,7 +3804,7 @@ sli_cmd_common_write_object(struct sli4 *sli4, void *buf, u16 noc, > wr_obj->desired_write_len_dword = cpu_to_le32(dwflags); > > wr_obj->write_offset = cpu_to_le32(offset); > - strncpy(wr_obj->object_name, obj_name, sizeof(wr_obj->object_name) - 1); > + strscpy(wr_obj->object_name, obj_name); Standard question for these kinds of conversions: Why is it safe that this is not NUL padded? I haven't found where this buffer is being zeroed out, but it probably is (given the "- 1" on the length), but without run-time testing, this needs much more careful analysis. -Kees > wr_obj->host_buffer_descriptor_count = cpu_to_le32(1); > > bde = (struct sli4_bde *)wr_obj->host_buffer_descriptor; > @@ -3833,7 +3833,7 @@ sli_cmd_common_delete_object(struct sli4 *sli4, void *buf, char *obj_name) > SLI4_SUBSYSTEM_COMMON, CMD_V0, > SLI4_RQST_PYLD_LEN(cmn_delete_object)); > > - strncpy(req->object_name, obj_name, sizeof(req->object_name) - 1); > + strscpy(req->object_name, obj_name); > return 0; > } > > @@ -3856,7 +3856,7 @@ sli_cmd_common_read_object(struct sli4 *sli4, void *buf, u32 desired_read_len, > cpu_to_le32(desired_read_len & SLI4_REQ_DESIRE_READLEN); > > rd_obj->read_offset = cpu_to_le32(offset); > - strncpy(rd_obj->object_name, obj_name, sizeof(rd_obj->object_name) - 1); > + strscpy(rd_obj->object_name, obj_name); > rd_obj->host_buffer_descriptor_count = cpu_to_le32(1); > > bde = (struct sli4_bde *)rd_obj->host_buffer_descriptor; > -- > 2.48.1 >
On 7. Apr 2025, at 20:28, Kees Cook wrote: > On Wed, Feb 26, 2025 at 07:55:26PM +0100, Thorsten Blum wrote: >> strncpy() is deprecated for NUL-terminated destination buffers; use >> strscpy() instead. >> >> Compile-tested only. >> >> Link: https://github.com/KSPP/linux/issues/90 >> Cc: linux-hardening@vger.kernel.org >> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev> >> --- > > Standard question for these kinds of conversions: Why is it safe that > this is not NUL padded? I haven't found where this buffer is being > zeroed out, but it probably is (given the "- 1" on the length), but > without run-time testing, this needs much more careful analysis. I think this was submitted before I started to explain this better. 'wr_obj' is the zeroed out 'buf' returned from sli_config_cmd_init(). I'll update the description and submit a v2. Thanks, Thorsten
On Mon, Apr 07, 2025 at 09:01:53PM +0200, Thorsten Blum wrote: > On 7. Apr 2025, at 20:28, Kees Cook wrote: > > On Wed, Feb 26, 2025 at 07:55:26PM +0100, Thorsten Blum wrote: > >> strncpy() is deprecated for NUL-terminated destination buffers; use > >> strscpy() instead. > >> > >> Compile-tested only. > >> > >> Link: https://github.com/KSPP/linux/issues/90 > >> Cc: linux-hardening@vger.kernel.org > >> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev> > >> --- > > > > Standard question for these kinds of conversions: Why is it safe that > > this is not NUL padded? I haven't found where this buffer is being > > zeroed out, but it probably is (given the "- 1" on the length), but > > without run-time testing, this needs much more careful analysis. > > I think this was submitted before I started to explain this better. > > 'wr_obj' is the zeroed out 'buf' returned from sli_config_cmd_init(). I don't see how dma->virt and buf are associated? > > I'll update the description and submit a v2. Thanks! > > Thanks, > Thorsten >
On 7. Apr 2025, at 22:30, Kees Cook wrote: > On Mon, Apr 07, 2025 at 09:01:53PM +0200, Thorsten Blum wrote: >> On 7. Apr 2025, at 20:28, Kees Cook wrote: >>> On Wed, Feb 26, 2025 at 07:55:26PM +0100, Thorsten Blum wrote: >>>> strncpy() is deprecated for NUL-terminated destination buffers; use >>>> strscpy() instead. >>>> >>>> Compile-tested only. >>>> >>>> Link: https://github.com/KSPP/linux/issues/90 >>>> Cc: linux-hardening@vger.kernel.org >>>> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev> >>>> --- >>> >>> Standard question for these kinds of conversions: Why is it safe that >>> this is not NUL padded? I haven't found where this buffer is being >>> zeroed out, but it probably is (given the "- 1" on the length), but >>> without run-time testing, this needs much more careful analysis. >> >> I think this was submitted before I started to explain this better. >> >> 'wr_obj' is the zeroed out 'buf' returned from sli_config_cmd_init(). > > I don't see how dma->virt and buf are associated? Since dma is NULL, sli_config_cmd_init() returns config->payload.embed early (it doesn't get to return dma->virt) and before we have memset(buf, 0, SLI4_BMBX_SIZE); config = buf; and SLI4_BMBX_SIZE is 256 which matches the size of sli4_cmd_sli_config. It's not very obvious tbh.
diff --git a/drivers/scsi/elx/libefc_sli/sli4.c b/drivers/scsi/elx/libefc_sli/sli4.c index 5e7fb110bc3f..d9a231fc0e0d 100644 --- a/drivers/scsi/elx/libefc_sli/sli4.c +++ b/drivers/scsi/elx/libefc_sli/sli4.c @@ -3804,7 +3804,7 @@ sli_cmd_common_write_object(struct sli4 *sli4, void *buf, u16 noc, wr_obj->desired_write_len_dword = cpu_to_le32(dwflags); wr_obj->write_offset = cpu_to_le32(offset); - strncpy(wr_obj->object_name, obj_name, sizeof(wr_obj->object_name) - 1); + strscpy(wr_obj->object_name, obj_name); wr_obj->host_buffer_descriptor_count = cpu_to_le32(1); bde = (struct sli4_bde *)wr_obj->host_buffer_descriptor; @@ -3833,7 +3833,7 @@ sli_cmd_common_delete_object(struct sli4 *sli4, void *buf, char *obj_name) SLI4_SUBSYSTEM_COMMON, CMD_V0, SLI4_RQST_PYLD_LEN(cmn_delete_object)); - strncpy(req->object_name, obj_name, sizeof(req->object_name) - 1); + strscpy(req->object_name, obj_name); return 0; } @@ -3856,7 +3856,7 @@ sli_cmd_common_read_object(struct sli4 *sli4, void *buf, u32 desired_read_len, cpu_to_le32(desired_read_len & SLI4_REQ_DESIRE_READLEN); rd_obj->read_offset = cpu_to_le32(offset); - strncpy(rd_obj->object_name, obj_name, sizeof(rd_obj->object_name) - 1); + strscpy(rd_obj->object_name, obj_name); rd_obj->host_buffer_descriptor_count = cpu_to_le32(1); bde = (struct sli4_bde *)rd_obj->host_buffer_descriptor;
strncpy() is deprecated for NUL-terminated destination buffers; use strscpy() instead. Compile-tested only. Link: https://github.com/KSPP/linux/issues/90 Cc: linux-hardening@vger.kernel.org Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev> --- drivers/scsi/elx/libefc_sli/sli4.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)