[PATCH v2] nvmet: replace strncpy with strscpy

[PATCH v2] nvmet: replace strncpy with strscpy

[Marcelo Moreira]

The strncpy() function is deprecated for NUL-terminated strings as explained in
the "strncpy() on NUL-terminated strings" section of
Documentation/process/deprecated.rst. The key issues are:
- strncpy() fails to guarantee NULL-termination when source > destination.
- It unnecessarily zero-pads short strings, causing performance overhead.

strscpy() is the proper replacement because:
- Guarantees NULL-termination.
- Avoids redundant zero-padding.
- Aligns with current kernel string-copying best practice.

memcpy() was rejected because:
- NQN buffers (subsysnqn/hostnqn) are treated as NULL-terminated strings:
  - strcmp() usage in nvmet_host_allowed() (discovery.c)
  - strscpy() to copy subsysnqn in nvmet_execute_disc_identify()

seq_buf wasn't used because:
- This is a simple fixed-size buffer copy.
- No need for progressive string construction features.

Signed-off-by: Marcelo Moreira <marcelomoreira1905@gmail.com>
---
Changes in v2:

- Added references to where to find deprecated strncpy.
- Commit message with detailed rationale for choosing strscpy.
- Added explicit rejection reasons for memcpy and seq_buf alternatives.
- No code changes from v1.
- Link to v1: https://lore.kernel.org/linux-nvme/20250403042737.GE22360@lst.de/T/#t
---
 drivers/nvme/target/discovery.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/nvme/target/discovery.c b/drivers/nvme/target/discovery.c
index df7207640506..c06f3e04296c 100644
--- a/drivers/nvme/target/discovery.c
+++ b/drivers/nvme/target/discovery.c
@@ -119,7 +119,7 @@ static void nvmet_format_discovery_entry(struct nvmf_disc_rsp_page_hdr *hdr,
 	memcpy(e->trsvcid, port->disc_addr.trsvcid, NVMF_TRSVCID_SIZE);
 	memcpy(e->traddr, traddr, NVMF_TRADDR_SIZE);
 	memcpy(e->tsas.common, port->disc_addr.tsas.common, NVMF_TSAS_SIZE);
-	strncpy(e->subnqn, subsys_nqn, NVMF_NQN_SIZE);
+	strscpy(e->subnqn, subsys_nqn, NVMF_NQN_SIZE);
 }
 
 /*
-- 
2.49.0

Re: [PATCH v2] nvmet: replace strncpy with strscpy

[Chaitanya Kulkarni]

On 4/3/25 16:23, Marcelo Moreira wrote:
> The strncpy() function is deprecated for NUL-terminated strings as explained in
> the "strncpy() on NUL-terminated strings" section of
> Documentation/process/deprecated.rst. The key issues are:
> - strncpy() fails to guarantee NULL-termination when source > destination.
> - It unnecessarily zero-pads short strings, causing performance overhead.
>
> strscpy() is the proper replacement because:
> - Guarantees NULL-termination.
> - Avoids redundant zero-padding.
> - Aligns with current kernel string-copying best practice.
>
> memcpy() was rejected because:
> - NQN buffers (subsysnqn/hostnqn) are treated as NULL-terminated strings:
>    - strcmp() usage in nvmet_host_allowed() (discovery.c)
>    - strscpy() to copy subsysnqn in nvmet_execute_disc_identify()
>
> seq_buf wasn't used because:
> - This is a simple fixed-size buffer copy.
> - No need for progressive string construction features.
>
> Signed-off-by: Marcelo Moreira<marcelomoreira1905@gmail.com>

Looks good.

Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>

-ck

Re: [PATCH v2] nvmet: replace strncpy with strscpy

[Sagi Grimberg]

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

Re: [PATCH v2] nvmet: replace strncpy with strscpy

[Christoph Hellwig]

Thanks,

applied to nvme-6.16.