Hello,
I have posted this question to kernelnewbies-ml,
but I post here too, for this is a bit difficult question.
I have several questions about sysctl().
Q1: Why does sysctl_string() return 0 when it succeed?
The sysctl_string() do r/w operations,
so we needn't to proceed with automatic r/w
when sysctl_string() is called as
a strategy routine (i.e. table->strategy).
http://lxr.linux.no/source/kernel/sysctl.c#L1087
Q2: If Q1 is not a bug, then I have Q2.
Why not terminate with '\0' if doing string copy operation
at http://lxr.linux.no/source/kernel/sysctl.c#L1115 ?
At least, as with table->strategy == &sysctl_string ,
'\0' should be always added after string copy operation.
For example, /proc/sys/kernel/hostname uses
sysctl_string() for strategy routine.
http://lxr.linux.no/source/kernel/sysctl.c#L249
Since sysctl_string() returns 0, do_sysctl_strategy()
continues and proceeds with automatic r/w.
But since automatic r/w doesn't set trailing '\0',
table->data doesn't end with '\0' if newlen > table->maxlen.
This makes system_utsname.nodename not null-terminated.
Q3: The parse_table() call table->strategy() if
table->child != NULL && table->strategy != NULL .
http://lxr.linux.no/source/kernel/sysctl.c#L1049
Can such table (table->child != NULL && table->strategy != NULL) exist?
If such table can exist, table->strategy() is called
without ctl_perm(table, r/w) checks,
allowing non-root process read/write that table.
We need to call ctl_perm() before calling table->strategy(),
aren't we?
If such table can't exist, we can remove
"if (table->strategy) { ... }" block
(from http://lxr.linux.no/source/kernel/sysctl.c#L1049
to http://lxr.linux.no/source/kernel/sysctl.c#L1056 ),
aren't we?
Q4: do_sysctl() gets lock_kernel() but no uts_sem lock
( http://lxr.linux.no/source/kernel/sysctl.c#L1001 ), and
sys_sethostname() gets uts_sem lock but no lock_kernel()
( http://lxr.linux.no/source/kernel/sys.c#L1376 ).
This allows one process update hostname via sys_sysctl()
and the other process update hostname via sys_sethostname()
at the same time.
As with hostname, the race condition probably won't crash the system.
But there MAY be more sysctl-accessible variables
that can produce race condition.
Why not review all sysctl-accessible variables?
Best Regards.
----- demo code for Q2 start -----
#include <stdio.h>
#include <string.h>
#include <unistd.h>
#include <linux/unistd.h>
#include <linux/types.h>
#include <linux/sysctl.h>
#include <errno.h>
_syscall1(int, _sysctl, struct __sysctl_args *, args);
static int sysctl(int *name, int nlen, void *oldval, size_t *oldlenp, void *newval, size_t newlen) {
struct __sysctl_args args={name,nlen,oldval,oldlenp,newval,newlen};
return _sysctl(&args);
}
int main(int argc, char *argv[]) {
int name[] = { CTL_KERN, KERN_NODENAME };
static char buffer[128];
memset(buffer, 0, sizeof(buffer));
snprintf(buffer, sizeof(buffer) - 1, "12345678901234567890123456789012345678901234567890123456789012345");
if (sysctl(name, 2, NULL, 0, buffer, sizeof(buffer))) perror("sysctl");
else printf("hostname was set via sysctl()\n");
return 0;
}
----- demo code for Q2 end -----
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
