[PATCH] Fix user data corrupted by old value return of sysctl



If the user reads a sysctl entry which is of string type
 by sysctl syscall, this call probably corrupts the user data
 right after the old value buffer, the issue lies in sysctl_string
 seting 0 to oldval[len], len is the available buffer size
 specified by the user, obviously, this will write to the first
 byte of the user memory place immediate after the old value buffer
, the correct way is that sysctl_string doesn't set 0, the user
should do it by self in the program.

The following program verifies this point:

#include <linux/unistd.h>
#include <linux/types.h>
#include <linux/sysctl.h>
#include <errno.h>

_syscall1(int, _sysctl, struct __sysctl_args *, args);
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);
}

#define SIZE(x) sizeof(x)/sizeof(x[0])
#define OSNAMESZ 4

struct mystruct {
	char osname[OSNAMESZ];
	int target;
	int osnamelth;
} myos;

int name[] = { CTL_KERN, KERN_NODENAME };

int main(int argc, char * argv[])
{
	myos.target = 1;
	printf("target = %d\n", myos.target);
	myos.osnamelth = SIZE(myos.osname);
        if (sysctl(name, SIZE(name), myos.osname,
			&myos.osnamelth, 0, 0))
                perror("sysctl");
	else {
                printf("Current host name: %s\n", myos.osname);
	}
	printf("target = %d\n", myos.target);
        return 0;
}

Copy it to file sysctl-safe.c, then
$ hostname
mylocalmachine
$ gcc sysctl-safe.c
$ ./a.out
target = 1
Current host name: mylo
target = 0
$

After apply this patch:
$ hostname
mylocalmachine
$ gcc sysctl-safe.c
$ ./a.out
target = 1
Current host name: mylo
target = 1

Signed-off-by: Yi Yang <yang.y.yi@xxxxxxxxx>


--- a/kernel/sysctl.c.orig 2005-12-30 09:21:34.000000000 +0000 +++ b/kernel/sysctl.c 2005-12-30 15:58:15.000000000 +0000 @@ -2207,8 +2207,6 @@ int sysctl_string(ctl_table *table, int len = table->maxlen; if(copy_to_user(oldval, table->data, len)) return -EFAULT; - if(put_user(0, ((char __user *) oldval) + len)) - return -EFAULT; if(put_user(len, oldlenp)) return -EFAULT; }



-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/



Relevant Pages

  • Re: [PATCH] Fix user data corrupted by old value return of sysctl
    ... by sysctl syscall, this call probably corrupts the user data right after the old value buffer, the issue lies in sysctl_string seting 0 to oldval, len is the available buffer size specified by the user, obviously, this will write to the first byte of the user memory place immediate after the old value buffer, the correct way is that sysctl_string doesn't set 0, the user should do it by self in the program. ... int sysctl{struct __sysctl_args args ... int target; ... Current host name: ...
    (Linux-Kernel)
  • [PATCH 01/24] Fix setting of PF_SUPERPRIV by __capable() [ver #7]
    ... Fix the setting of PF_SUPERPRIV by __capableas it could corrupt the flags ... the target process if that is not the current process and it is trying to ... * Return true if the specified task has the given superior capability ... int (struct task_struct *target, ...
    (Linux-Kernel)
  • [PATCH] Fix setting of PF_SUPERPRIV by __capable()
    ... Fix the setting of PF_SUPERPRIV by __capableas it could corrupt the flags ... the target process if that is not the current process and it is trying to ... * Return true if the specified task has the given superior capability ... int (struct task_struct *target, ...
    (Linux-Kernel)
  • [PATCH] Fix setting of PF_SUPERPRIV by __capable()
    ... Fix the setting of PF_SUPERPRIV by __capableas it could corrupt the flags ... the target process if that is not the current process and it is trying to ... * Return true if the specified task has the given superior capability ... int (struct task_struct *target, ...
    (Linux-Kernel)
  • Re: Two macros for resource management
    ... > PUSH(fclose(source)) ... > PUSH(fclose(target)) ... > PUSH(free(buffer)) ... {int result = EXIT_FAILURE; ...
    (comp.lang.c)