All of lore.kernel.org
 help / color / mirror / Atom feed
From: Justin Stitt <justinstitt@google.com>
To: Baoquan He <bhe@redhat.com>, Vivek Goyal <vgoyal@redhat.com>,
	Dave Young <dyoung@redhat.com>
Cc: kexec@lists.infradead.org, linux-kernel@vger.kernel.org,
	 linux-fsdevel@vger.kernel.org, linux-hardening@vger.kernel.org,
	 Justin Stitt <justinstitt@google.com>
Subject: [PATCH] vmcore: replace strncpy with strtomem
Date: Wed, 27 Mar 2024 21:10:52 +0000	[thread overview]
Message-ID: <20240327-strncpy-fs-proc-vmcore-c-v1-1-e025ed08b1b0@google.com> (raw)

strncpy() is in the process of being replaced as it is deprecated in
some situations [1]. While the specific use of strncpy that this patch
targets is not exactly deprecated, the real mission is to rid the kernel
of all its uses.

Looking at vmcoredd_header's definition:
|	struct vmcoredd_header {
|		__u32 n_namesz; /* Name size */
|		__u32 n_descsz; /* Content size */
|		__u32 n_type;   /* NT_VMCOREDD */
|		__u8 name[8];   /* LINUX\0\0\0 */
|		__u8 dump_name[VMCOREDD_MAX_NAME_BYTES]; /* Device dump's name */
|	};
... we can see that both `name` and `dump_name` are u8s. It seems `name`
wants to be NUL-padded (based on the comment above), but for the sake of
symmetry lets NUL-pad both of these.

Mark these buffers as __nonstring and use strtomem_pad.

Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1]
Link: https://github.com/KSPP/linux/issues/90
Cc: linux-hardening@vger.kernel.org
Signed-off-by: Justin Stitt <justinstitt@google.com>
---
Note: build-tested only.

Found with: $ rg "strncpy\("
---
 fs/proc/vmcore.c            | 5 ++---
 include/uapi/linux/vmcore.h | 4 ++--
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/fs/proc/vmcore.c b/fs/proc/vmcore.c
index 1fb213f379a5..5d7ecf3b75e8 100644
--- a/fs/proc/vmcore.c
+++ b/fs/proc/vmcore.c
@@ -1370,9 +1370,8 @@ static void vmcoredd_write_header(void *buf, struct vmcoredd_data *data,
 	vdd_hdr->n_descsz = size + sizeof(vdd_hdr->dump_name);
 	vdd_hdr->n_type = NT_VMCOREDD;
 
-	strncpy((char *)vdd_hdr->name, VMCOREDD_NOTE_NAME,
-		sizeof(vdd_hdr->name));
-	memcpy(vdd_hdr->dump_name, data->dump_name, sizeof(vdd_hdr->dump_name));
+	strtomem_pad(vdd_hdr->name, VMCOREDD_NOTE_NAME, 0);
+	strtomem_pad(vdd_hdr->dump_name, data->dump_name, 0);
 }
 
 /**
diff --git a/include/uapi/linux/vmcore.h b/include/uapi/linux/vmcore.h
index 3e9da91866ff..7053e2b62fa0 100644
--- a/include/uapi/linux/vmcore.h
+++ b/include/uapi/linux/vmcore.h
@@ -11,8 +11,8 @@ struct vmcoredd_header {
 	__u32 n_namesz; /* Name size */
 	__u32 n_descsz; /* Content size */
 	__u32 n_type;   /* NT_VMCOREDD */
-	__u8 name[8];   /* LINUX\0\0\0 */
-	__u8 dump_name[VMCOREDD_MAX_NAME_BYTES]; /* Device dump's name */
+	__u8 name[8] __nonstring;   /* LINUX\0\0\0 */
+	__u8 dump_name[VMCOREDD_MAX_NAME_BYTES] __nonstring; /* Device dump's name */
 };
 
 #endif /* _UAPI_VMCORE_H */

---
base-commit: 928a87efa42302a23bb9554be081a28058495f22
change-id: 20240327-strncpy-fs-proc-vmcore-c-b18d761feaef

Best regards,
--
Justin Stitt <justinstitt@google.com>


WARNING: multiple messages have this Message-ID (diff)
From: Justin Stitt <justinstitt@google.com>
To: Baoquan He <bhe@redhat.com>, Vivek Goyal <vgoyal@redhat.com>,
	Dave Young <dyoung@redhat.com>
Cc: kexec@lists.infradead.org, linux-kernel@vger.kernel.org,
	 linux-fsdevel@vger.kernel.org, linux-hardening@vger.kernel.org,
	 Justin Stitt <justinstitt@google.com>
Subject: [PATCH] vmcore: replace strncpy with strtomem
Date: Wed, 27 Mar 2024 21:10:52 +0000	[thread overview]
Message-ID: <20240327-strncpy-fs-proc-vmcore-c-v1-1-e025ed08b1b0@google.com> (raw)

strncpy() is in the process of being replaced as it is deprecated in
some situations [1]. While the specific use of strncpy that this patch
targets is not exactly deprecated, the real mission is to rid the kernel
of all its uses.

Looking at vmcoredd_header's definition:
|	struct vmcoredd_header {
|		__u32 n_namesz; /* Name size */
|		__u32 n_descsz; /* Content size */
|		__u32 n_type;   /* NT_VMCOREDD */
|		__u8 name[8];   /* LINUX\0\0\0 */
|		__u8 dump_name[VMCOREDD_MAX_NAME_BYTES]; /* Device dump's name */
|	};
... we can see that both `name` and `dump_name` are u8s. It seems `name`
wants to be NUL-padded (based on the comment above), but for the sake of
symmetry lets NUL-pad both of these.

Mark these buffers as __nonstring and use strtomem_pad.

Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1]
Link: https://github.com/KSPP/linux/issues/90
Cc: linux-hardening@vger.kernel.org
Signed-off-by: Justin Stitt <justinstitt@google.com>
---
Note: build-tested only.

Found with: $ rg "strncpy\("
---
 fs/proc/vmcore.c            | 5 ++---
 include/uapi/linux/vmcore.h | 4 ++--
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/fs/proc/vmcore.c b/fs/proc/vmcore.c
index 1fb213f379a5..5d7ecf3b75e8 100644
--- a/fs/proc/vmcore.c
+++ b/fs/proc/vmcore.c
@@ -1370,9 +1370,8 @@ static void vmcoredd_write_header(void *buf, struct vmcoredd_data *data,
 	vdd_hdr->n_descsz = size + sizeof(vdd_hdr->dump_name);
 	vdd_hdr->n_type = NT_VMCOREDD;
 
-	strncpy((char *)vdd_hdr->name, VMCOREDD_NOTE_NAME,
-		sizeof(vdd_hdr->name));
-	memcpy(vdd_hdr->dump_name, data->dump_name, sizeof(vdd_hdr->dump_name));
+	strtomem_pad(vdd_hdr->name, VMCOREDD_NOTE_NAME, 0);
+	strtomem_pad(vdd_hdr->dump_name, data->dump_name, 0);
 }
 
 /**
diff --git a/include/uapi/linux/vmcore.h b/include/uapi/linux/vmcore.h
index 3e9da91866ff..7053e2b62fa0 100644
--- a/include/uapi/linux/vmcore.h
+++ b/include/uapi/linux/vmcore.h
@@ -11,8 +11,8 @@ struct vmcoredd_header {
 	__u32 n_namesz; /* Name size */
 	__u32 n_descsz; /* Content size */
 	__u32 n_type;   /* NT_VMCOREDD */
-	__u8 name[8];   /* LINUX\0\0\0 */
-	__u8 dump_name[VMCOREDD_MAX_NAME_BYTES]; /* Device dump's name */
+	__u8 name[8] __nonstring;   /* LINUX\0\0\0 */
+	__u8 dump_name[VMCOREDD_MAX_NAME_BYTES] __nonstring; /* Device dump's name */
 };
 
 #endif /* _UAPI_VMCORE_H */

---
base-commit: 928a87efa42302a23bb9554be081a28058495f22
change-id: 20240327-strncpy-fs-proc-vmcore-c-b18d761feaef

Best regards,
--
Justin Stitt <justinstitt@google.com>


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

             reply	other threads:[~2024-03-27 21:10 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-27 21:10 Justin Stitt [this message]
2024-03-27 21:10 ` [PATCH] vmcore: replace strncpy with strtomem Justin Stitt
2024-03-28  1:48 ` Baoquan He
2024-03-28  1:48   ` Baoquan He
2024-03-28 20:42   ` Justin Stitt
2024-03-28 20:42     ` Justin Stitt
2024-03-28 22:03 ` Kees Cook
2024-03-28 22:03   ` Kees Cook

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=20240327-strncpy-fs-proc-vmcore-c-v1-1-e025ed08b1b0@google.com \
    --to=justinstitt@google.com \
    --cc=bhe@redhat.com \
    --cc=dyoung@redhat.com \
    --cc=kexec@lists.infradead.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-hardening@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=vgoyal@redhat.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.