CODEWITHSUNDEEP
clinuxkernel
laoqiren
laoqiren

Reputation: 3877

copy data from linux kernel space to user space

I'm adding a kernel function to modify the nice and prio value of Specified pid of process.

Here is the code:

SYSCALL_DEFINE5(mysetnice, pid_t, pid, int, flag, int, nicevalue, void __user *, prio, void __user *, nice)
{
    if (pid < 0 || (flag != 1 && flag != 0)) {
        return EFAULT;
    }
    if (nice == NULL || prio == NULL) {
        return EFAULT;
    }
    struct task_struct *p;
    for_each_process(p) {
        if (p->pid == pid) {
            if (flag == 1) {
                set_user_nice(p, nicevalue);
                printk("change nice=%d of pid=%d\n", nicevalue, pid);
            }
            printk("pid:%d,current nice:%d, prio:%d\n",pid,task_nice(p),task_prio(p));
            copy_to_user(prio,(const void*)task_prio(p),sizeof(int));
            copy_to_user(nice,(const void*)task_nice(p),sizeof(int));
            return 0;
        }
    }
    return EFAULT;

}

and in user scope, I write a test function like this:

int main()
{
    int pid = 0;
    int flag = 0;
    int nicevalue = 0;
    int prio = 0;
    int nice = 0;
    int ret;

    printf("please input the params: pid, flag, nicevalue\n");
    while(~scanf("%d %d %d", &pid, &flag, &nicevalue))
    {
        ret = syscall(__NR_mysyscall, pid, flag, nicevalue, (void *)&prio, (void *)&nice);
        if (ret != 0)
        {
            printf("syscall error! error code:%d\n", ret);
            return 0;
        }
        printf("pid:%d, flag:%d, nicevalue:%d, prio:%d, nice:%d\n", pid, flag, nicevalue, prio + 100, nice);
    }

    return 0;
}

but I found that the I can't get the nice value, and the prio is always 100. So how to copy data from kernel scope to user scope?

Upvotes: 2

Views: 1121

Answers (2)

user4822941
user4822941

Reputation:

The code is of very poor quality in general.

SYSCALL_DEFINE5(mysetnice, pid_t, pid, int, flag, int, nicevalue, void __user *, prio, void __user *, nice)
{
    if (pid < 0 || (flag != 1 && flag != 0)) {
        return EFAULT;

Error codes are negative. I.e. this should be -EFAULT, as can be seen in other syscalls. EFAULT itself is an extremely poor choice for this particular condition. Normally invalid arguments result in EINVAL.

    }
    if (nice == NULL || prio == NULL) {
        return EFAULT;
    }
    struct task_struct *p;
    for_each_process(p) {

Where did you take this from?

There is 0 reason to walk the process list. Finding a way to translate pid -> task_struct * is left as an exercise. Hint: there are syscalls which take pid as an argument.

The traversal itself is incorrect due to insufficient (or rather: lack of) locking.

        if (p->pid == pid) {
            if (flag == 1) {
                set_user_nice(p, nicevalue);
                printk("change nice=%d of pid=%d\n", nicevalue, pid);
            }
            printk("pid:%d,current nice:%d, prio:%d\n",pid,task_nice(p),task_prio(p));

Lack of log level in printk. Check any existing printk to see what I mean.

            copy_to_user(prio,(const void*)task_prio(p),sizeof(int));

Not only no error checking but obviously incorrect cast of the second argument.

            copy_to_user(nice,(const void*)task_nice(p),sizeof(int));
            return 0;
        }
    }
    return EFAULT;

This one should be ESRCH.

}

Upvotes: 3

Ctx
Ctx

Reputation: 18410

The problem is, that you have to provide the address of the source value, not the value itself. Try something like:

int val = task_prio(p);
copy_to_user(prio,(const void*)&val,sizeof(int));

same for nice, then it should work as expected.

Upvotes: 2

Related Questions