[PATCH] add setutsname to reuse code in setdomainname and sethostname

[PATCH] add setutsname to reuse code in setdomainname and sethostname

[Laurijssen]

- sethostname and setdomainname duplicated code with one field difference.
- added setutsname with arguments domainname/hostname to reuse the same code.

Signed-off-by: Laurijssen <laurijssen@protonmail.com>
---
 kernel/sys.c | 46 ++++++++++++++++++----------------------------
 1 file changed, 18 insertions(+), 28 deletions(-)

diff --git a/kernel/sys.c b/kernel/sys.c
index 755bfa5ccd92..3a4e483c22d5 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -1419,7 +1419,9 @@ SYSCALL_DEFINE1(olduname, struct oldold_utsname __user *, name)
 }
 #endif
 
-SYSCALL_DEFINE2(sethostname, char __user *, name, int, len)
+static int setutsname(char __user *name, int len, enum uts_proc uts_proc,
+		      char (*field)[__NEW_UTS_LEN + 1],
+		      int fieldlen)
 {
 	int errno;
 	char tmp[__NEW_UTS_LEN];
@@ -1431,20 +1433,26 @@ SYSCALL_DEFINE2(sethostname, char __user *, name, int, len)
 		return -EINVAL;
 	errno = -EFAULT;
 	if (!copy_from_user(tmp, name, len)) {
-		struct new_utsname *u;
-
 		add_device_randomness(tmp, len);
 		down_write(&uts_sem);
-		u = utsname();
-		memcpy(u->nodename, tmp, len);
-		memset(u->nodename + len, 0, sizeof(u->nodename) - len);
+		memcpy(*field, tmp, len);
+		memset(*field + len, 0, fieldlen - len);
+
 		errno = 0;
-		uts_proc_notify(UTS_PROC_HOSTNAME);
+		uts_proc_notify(uts_proc);
 		up_write(&uts_sem);
 	}
 	return errno;
 }
 
+SYSCALL_DEFINE2(sethostname, char __user *, name, int, len)
+{
+	// utsname() always returns a valid pointer to the current utsname structure.
+	struct new_utsname *u = utsname();
+
+	return setutsname(name, len, UTS_PROC_HOSTNAME, &u->nodename, sizeof(u->nodename));
+}
+
 #ifdef __ARCH_WANT_SYS_GETHOSTNAME
 
 SYSCALL_DEFINE2(gethostname, char __user *, name, int, len)
@@ -1475,28 +1483,10 @@ SYSCALL_DEFINE2(gethostname, char __user *, name, int, len)
  */
 SYSCALL_DEFINE2(setdomainname, char __user *, name, int, len)
 {
-	int errno;
-	char tmp[__NEW_UTS_LEN];
-
-	if (!ns_capable(current->nsproxy->uts_ns->user_ns, CAP_SYS_ADMIN))
-		return -EPERM;
-	if (len < 0 || len > __NEW_UTS_LEN)
-		return -EINVAL;
-
-	errno = -EFAULT;
-	if (!copy_from_user(tmp, name, len)) {
-		struct new_utsname *u;
+	// utsname() always returns a valid pointer to the current utsname structure.
+	struct new_utsname *u = utsname();
 
-		add_device_randomness(tmp, len);
-		down_write(&uts_sem);
-		u = utsname();
-		memcpy(u->domainname, tmp, len);
-		memset(u->domainname + len, 0, sizeof(u->domainname) - len);
-		errno = 0;
-		uts_proc_notify(UTS_PROC_DOMAINNAME);
-		up_write(&uts_sem);
-	}
-	return errno;
+	return setutsname(name, len, UTS_PROC_DOMAINNAME, &u->domainname, sizeof(u->domainname));
 }
 
 /* make sure you are allowed to change @tsk limits before calling this */
-- 
2.53.0

Re: [PATCH] add setutsname to reuse code in setdomainname and sethostname

[Bradley Morgan]

> +static int setutsname(char __user *name, int len, enum uts_proc uts_proc,
> +		      char (*field)[__NEW_UTS_LEN + 1],
> +		      int fieldlen)

If we pass uts_proc, why do we also need field? Nothing keeps them in sync.
UTS_PROC_HOSTNAME paired with &u->domainname still writes where you wanted,
it just notifies the wrong sysctl poll, and the compiler says nothing about
it. Both callsites are right today so its not a bug, its a shape I would
rather not have in the tree. The enum already names the two writable
fields,
so let the helper pick the field from it. fieldlen can go too, the array in
the parameter list already says 65, so sizeof(*field) is the whole field. I
built that bit standalone to be sure, it gives 65.

Diff on top of your patch, it applies clean and builds here:

diff --git a/kernel/sys.c b/kernel/sys.c
index 3a4e483..f957871 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -1419,9 +1419,7 @@ SYSCALL_DEFINE1(olduname, struct oldold_utsname __user *, name)
 }
 #endif
 
-static int setutsname(char __user *name, int len, enum uts_proc uts_proc,
-		      char (*field)[__NEW_UTS_LEN + 1],
-		      int fieldlen)
+static int setutsname(char __user *name, int len, enum uts_proc proc)
 {
 	int errno;
 	char tmp[__NEW_UTS_LEN];
@@ -1433,13 +1431,17 @@ static int setutsname(char __user *name, int len, enum uts_proc uts_proc,
 		return -EINVAL;
 	errno = -EFAULT;
 	if (!copy_from_user(tmp, name, len)) {
+		struct new_utsname *u;
+		char *field;
+
 		add_device_randomness(tmp, len);
 		down_write(&uts_sem);
-		memcpy(*field, tmp, len);
-		memset(*field + len, 0, fieldlen - len);
-
+		u = utsname();
+		field = proc == UTS_PROC_HOSTNAME ? u->nodename : u->domainname;
+		memcpy(field, tmp, len);
+		memset(field + len, 0, sizeof_field(struct new_utsname, nodename) - len);
 		errno = 0;
-		uts_proc_notify(uts_proc);
+		uts_proc_notify(proc);
 		up_write(&uts_sem);
 	}
 	return errno;
@@ -1447,10 +1449,7 @@ static int setutsname(char __user *name, int len, enum uts_proc uts_proc,
 
 SYSCALL_DEFINE2(sethostname, char __user *, name, int, len)
 {
-	// utsname() always returns a valid pointer to the current utsname structure.
-	struct new_utsname *u = utsname();
-
-	return setutsname(name, len, UTS_PROC_HOSTNAME, &u->nodename, sizeof(u->nodename));
+	return setutsname(name, len, UTS_PROC_HOSTNAME);
 }
 
 #ifdef __ARCH_WANT_SYS_GETHOSTNAME
@@ -1483,10 +1482,7 @@ SYSCALL_DEFINE2(gethostname, char __user *, name, int, len)
  */
 SYSCALL_DEFINE2(setdomainname, char __user *, name, int, len)
 {
-	// utsname() always returns a valid pointer to the current utsname structure.
-	struct new_utsname *u = utsname();
-
-	return setutsname(name, len, UTS_PROC_DOMAINNAME, &u->domainname, sizeof(u->domainname));
+	return setutsname(name, len, UTS_PROC_DOMAINNAME);
 }
 
 /* make sure you are allowed to change @tsk limits before calling this */

Then both syscalls are a one line return, 12 added lines against your 18.
If
you would rather the other procs be loud instead of silently taken as
domainname, make it a switch with WARN_ON_ONCE(1) in the default, I'd take
either form.


> +	// utsname() always returns a valid pointer to the current utsname structure.
> +	struct new_utsname *u = utsname();

I dont think reading utsname() outside the lock is unsafe, current owns its
nsproxy and nothing else can move uts_ns under it. But the hoist buys
nothing and it is a change your changelog does not mention: uts_sem used to
cover the call. Keeping it next to the memcpy means there is nothing to
explain. The comment restates the line it sits on and it appears twice, so
the diff drops it. Also kernel/sys.c has no // comments of its own, the
single // in it is the SPDX line, so it reads new for this file either way.
Anything that isnt needed, goes.

If you want more of a cleanup, the /proc/sys/kernel/
hostname writers in kernel/utsname_sysctl.c buffer, call
add_device_randomness(), take uts_sem and memcpy, so they are a third copy
of this. Not the same though, they write all 65 bytes rather than len plus
the zero fill, so afaics your patch doesn't try to cover them and that is
fine. But that file has had "Should this be in kernel/sys.c ????" sitting
over proc_do_uts_string() since 2007, so if you want the bigger version of
this cleanup, that is where the shared helper goes.
--- Thanks!
https://lore.kernel.org/all/EE579805-42F2-4C58-B752-F28779EEB717@grrlz.net/

[PATCH] SYS: add utsname_set_field to reuse code in setdomainname and sethostname

[Laurijssen]

Changes after review, cleanup of duplicated code in sethostname and
setdomainname. Moved reusable code to utsname_sysctl.c

- add utsname_set_field to utsname_sysctl.c
- call utsname_set_field from sethostname/setdomainname
- moved uts_proc_notify from .h into utsname_sysctl.c and turned static

Signed-off-by: Laurijssen <laurijssen@protonmail.com>
---
 include/linux/utsname.h | 10 ++----
 kernel/sys.c            | 46 ++-----------------------
 kernel/utsname_sysctl.c | 75 ++++++++++++++++++++++++++++++++++-------
 3 files changed, 66 insertions(+), 65 deletions(-)

diff --git a/include/linux/utsname.h b/include/linux/utsname.h
index dc5594ffffcc..409261d5381f 100644
--- a/include/linux/utsname.h
+++ b/include/linux/utsname.h
@@ -18,14 +18,6 @@ enum uts_proc {
 	UTS_PROC_DOMAINNAME,
 };
 
-#ifdef CONFIG_SYSCTL
-extern void uts_proc_notify(enum uts_proc proc);
-#else
-static inline void uts_proc_notify(enum uts_proc proc)
-{
-}
-#endif
-
 static inline struct new_utsname *utsname(void)
 {
 	return &current->nsproxy->uts_ns->name;
@@ -38,4 +30,6 @@ static inline struct new_utsname *init_utsname(void)
 
 extern struct rw_semaphore uts_sem;
 
+int utsname_set_field(char __user *name, int len, enum uts_proc proc);
+
 #endif /* _LINUX_UTSNAME_H */
diff --git a/kernel/sys.c b/kernel/sys.c
index 755bfa5ccd92..688c5e9e222f 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -1421,28 +1421,7 @@ SYSCALL_DEFINE1(olduname, struct oldold_utsname __user *, name)
 
 SYSCALL_DEFINE2(sethostname, char __user *, name, int, len)
 {
-	int errno;
-	char tmp[__NEW_UTS_LEN];
-
-	if (!ns_capable(current->nsproxy->uts_ns->user_ns, CAP_SYS_ADMIN))
-		return -EPERM;
-
-	if (len < 0 || len > __NEW_UTS_LEN)
-		return -EINVAL;
-	errno = -EFAULT;
-	if (!copy_from_user(tmp, name, len)) {
-		struct new_utsname *u;
-
-		add_device_randomness(tmp, len);
-		down_write(&uts_sem);
-		u = utsname();
-		memcpy(u->nodename, tmp, len);
-		memset(u->nodename + len, 0, sizeof(u->nodename) - len);
-		errno = 0;
-		uts_proc_notify(UTS_PROC_HOSTNAME);
-		up_write(&uts_sem);
-	}
-	return errno;
+	return utsname_set_field(name, len, UTS_PROC_HOSTNAME);
 }
 
 #ifdef __ARCH_WANT_SYS_GETHOSTNAME
@@ -1475,28 +1454,7 @@ SYSCALL_DEFINE2(gethostname, char __user *, name, int, len)
  */
 SYSCALL_DEFINE2(setdomainname, char __user *, name, int, len)
 {
-	int errno;
-	char tmp[__NEW_UTS_LEN];
-
-	if (!ns_capable(current->nsproxy->uts_ns->user_ns, CAP_SYS_ADMIN))
-		return -EPERM;
-	if (len < 0 || len > __NEW_UTS_LEN)
-		return -EINVAL;
-
-	errno = -EFAULT;
-	if (!copy_from_user(tmp, name, len)) {
-		struct new_utsname *u;
-
-		add_device_randomness(tmp, len);
-		down_write(&uts_sem);
-		u = utsname();
-		memcpy(u->domainname, tmp, len);
-		memset(u->domainname + len, 0, sizeof(u->domainname) - len);
-		errno = 0;
-		uts_proc_notify(UTS_PROC_DOMAINNAME);
-		up_write(&uts_sem);
-	}
-	return errno;
+	return utsname_set_field(name, len, UTS_PROC_DOMAINNAME);
 }
 
 /* make sure you are allowed to change @tsk limits before calling this */
diff --git a/kernel/utsname_sysctl.c b/kernel/utsname_sysctl.c
index 6c1522201e39..026e5c877a8b 100644
--- a/kernel/utsname_sysctl.c
+++ b/kernel/utsname_sysctl.c
@@ -5,6 +5,7 @@
  *  Author: Eric Biederman <ebiederm@xmision.com>
  */
 
+#include <linux/capability.h>
 #include <linux/export.h>
 #include <linux/uts.h>
 #include <linux/utsname.h>
@@ -12,6 +13,9 @@
 #include <linux/sysctl.h>
 #include <linux/wait.h>
 #include <linux/rwsem.h>
+#include <linux/uaccess.h>
+
+static const struct ctl_table uts_kern_table[];
 
 #ifdef CONFIG_SYSCTL
 
@@ -67,10 +71,68 @@ static int proc_do_uts_string(const struct ctl_table *table, int write,
 
 	return r;
 }
+
+/*
+ * Notify userspace about a change in a certain entry of uts_kern_table,
+ * identified by the parameter proc.
+ */
+static void uts_proc_notify(enum uts_proc proc)
+{
+	const struct ctl_table *table = &uts_kern_table[proc];
+
+	proc_sys_poll_notify(table->poll);
+}
 #else
 #define proc_do_uts_string NULL
+
+static inline void uts_proc_notify(enum uts_proc proc)
+{
+}
 #endif
 
+int utsname_set_field(char __user *name, int len, enum uts_proc proc)
+{
+	int errno;
+	char tmp[__NEW_UTS_LEN];
+
+	if (!ns_capable(current->nsproxy->uts_ns->user_ns, CAP_SYS_ADMIN))
+		return -EPERM;
+
+	if (len < 0 || len > __NEW_UTS_LEN)
+		return -EINVAL;
+	errno = -EFAULT;
+	if (!copy_from_user(tmp, name, len)) {
+		struct new_utsname *u;
+		char *field;
+
+		add_device_randomness(tmp, len);
+		down_write(&uts_sem);
+
+		u = utsname();
+
+		switch (proc) {
+		case UTS_PROC_HOSTNAME:
+			field = u->nodename;
+			break;
+		case UTS_PROC_DOMAINNAME:
+			field = u->domainname;
+			break;
+		default:
+			WARN_ON_ONCE(1);
+			field = u->domainname;
+			break;
+		}
+
+		memcpy(field, tmp, len);
+		memset(field + len, 0, sizeof_field(struct new_utsname, nodename) - len);
+
+		errno = 0;
+		uts_proc_notify(proc);
+		up_write(&uts_sem);
+	}
+	return errno;
+}
+
 static DEFINE_CTL_TABLE_POLL(hostname_poll);
 static DEFINE_CTL_TABLE_POLL(domainname_poll);
 
@@ -122,19 +184,6 @@ static const struct ctl_table uts_kern_table[] = {
 	},
 };
 
-#ifdef CONFIG_SYSCTL
-/*
- * Notify userspace about a change in a certain entry of uts_kern_table,
- * identified by the parameter proc.
- */
-void uts_proc_notify(enum uts_proc proc)
-{
-	const struct ctl_table *table = &uts_kern_table[proc];
-
-	proc_sys_poll_notify(table->poll);
-}
-#endif
-
 static int __init utsname_sysctl_init(void)
 {
 	register_sysctl("kernel", uts_kern_table);
-- 
2.53.0

Re: [PATCH] SYS: add utsname_set_field to reuse code in setdomainname and sethostname

[kernel test robot]

Hi Laurijssen,

kernel test robot noticed the following build errors:

[auto build test ERROR on akpm-mm/mm-everything]
[also build test ERROR on brauner-vfs/vfs.all linus/master v7.4-rc2 next-20261109]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Laurijssen/SYS-add-utsname_set_field-to-reuse-code-in-setdomainname-and-sethostname/20261109-124421
base:   https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
patch link:    https://lore.kernel.org/r/20261109114421.8414-1-laurijssen%40protonmail.com
patch subject: [PATCH] SYS: add utsname_set_field to reuse code in setdomainname and sethostname
config: arc-randconfig-1001-20261109 (https://download.01.org/0day-ci/archive/20261110/202611100455.BsA8k1Nb-lkp@intel.com/config)
compiler: arc-linux-gcc (GCC) 14.4.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20261110/202611100455.BsA8k1Nb-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202611100455.BsA8k1Nb-lkp@intel.com/

All errors (new ones prefixed by >>):

   arc-linux-ld: kernel/sys.o: in function `__do_sys_sethostname':
   kernel/sys.c:1424:(.text+0x2c40): undefined reference to `utsname_set_field'
>> arc-linux-ld: kernel/sys.c:1424:(.text+0x2c40): undefined reference to `utsname_set_field'
   arc-linux-ld: kernel/sys.o: in function `__do_sys_setdomainname':
   kernel/sys.c:1457:(.text+0x2c80): undefined reference to `utsname_set_field'
   arc-linux-ld: kernel/sys.c:1457:(.text+0x2c80): undefined reference to `utsname_set_field'

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki