How to use copy_from_user?

Question

ssize_t probchar_write(struct file *filp, 
    const char __user *data, size_t s, loff_t *off) {

    printk(KERN_DEBUG "Data> |%s|\n", data); // only for debug
    char chars[MAX_LENGHT];
    if(s > MAX_LENGHT)
        s = MAX_LENGHT;
    if (copy_from_user(chars, data, s)) {
        return -EFAULT;
    }
    printk(KERN_DEBUG "Chars> |%s|\n", chars);
    return s;
}

This is dmesg output

[66777.956582] Data> |45
[66777.956596] |
[66777.956634] Chars> |45
[66777.956636] �    Ҩ�H��   H�� |

Why the copied chain has more characters in the end?

the correct wording should be `MAX_LENGTH`, not `MAX_LENGHT` ;) — Cédric Julien, May 28 '11 at 21:31

Iustin · Accepted Answer · 2011-05-28T18:06:38.110

2

This seems to be a write function from a device driver.

First

printk(KERN_DEBUG "Data> |%s|\n", data);

Do not do this! Do not ever directly access user data ever!

~~Second~~

char chars[s];

I doubt this is legal C. You need to either specify a size at compile time, or use kmalloc.

The copy_from_user usage is good. You should check for error and return -EFAULT. This is ok.

So just try and allocating the chars and it should work. You might also want to look at the offset, though for academic purposes that can be initially skipped.

edited May 28 '11 at 18:06

answered May 28 '11 at 15:54

Iustin

1,220
1
12
17

Seems I need to catch up on my standard. The way he uses copy_from_user is then fine. – Iustin May 28 '11 at 16:07
printk(KERN_DEBUG "Data> |%s|\n", data); was only for debug Now I made the array with a fixed size, but the same thing keeps happening – caeycae May 28 '11 at 17:35
From what I see you are using it correctly. What are you doing in user space? – Iustin May 28 '11 at 17:54
Add a null terminator at the end of chars. chars[s] = '\0'; Alternatively you can make a program that opens /dev/probchar and write to it a null terminated string of size strlen(str). – Iustin May 28 '11 at 18:05
Tanks it works perfect now, but strict_strtoul dont validate a very big long? If I entrer echo "4543434443" > /dev/probchar it returns 248467147... – caeycae May 28 '11 at 21:48
I don't know about strtoul, but from what I searched it seems you may want to add another l at the end. That number seems to be bigger then 2^32(unsigned int is usually the same as long). It can fit however in a long long. Try strict_strtoull for that number. I am not guaranteeing it. – Iustin May 28 '11 at 21:59
It seems that 2^32 - 4543434443 = -248467147. So you need a bigger storage. Try long long. – Iustin May 28 '11 at 22:03
yes, my idea was validate the input number with strict_strtoul... no use a bigger storage – caeycae May 28 '11 at 22:08
Then pass it to strict_strtoull. Wouldn't that suffice? – Iustin May 28 '11 at 22:11
my idea was throw an error if the user enter a number greater than 2^32 Really thanks a lot! – caeycae May 28 '11 at 22:26
anyoue has an idea how to do it? – caeycae May 29 '11 at 17:32
Oh I thought you had everything cleared. Use what? You should think about making another question since I am probably the only one that still reads / receives alerts for this question. – Iustin May 29 '11 at 18:38

score 0 · Answer 2 · answered Sep 22 '11 at 14:08

Suppose the following: there is no \0 at the end of the provided string. You don't add one or enforce one to be present. Then you print the string after copying the "valid" data on a stack-allocated buffer that contains random garbage. That makes extra characters being printed.

Suggested check: allocate MAX_LENGTH+1 characters and after you copied the data, do chars[s]=0. You might want to strip that \n character that kills the formatting of your log, too.

How to use copy_from_user?

2 Answers2