I'm trying to allocate 开发者_如何学Csome memory for a char* as follows.
static ssize_t memo_write(struct file *filp, const char __user *buf,
size_t count, loff_t *f_pos){
ssize_t retval = -ENOMEM;
printk("write function\n");
if((data = kmalloc(strlen(buf), GFP_KERNEL)) == NULL)
printk("kmalloc fail\n");
if(copy_from_user(data, buf, strlen(buf))){
retval = -EFAULT;
goto out;
}
*f_pos += strlen(buf);
retval = strlen(buf);
out:
return retval;
}
'data' is declared in a header file as
char *data;
When I call the write function, the 'kmalloc fail' line isn't reached, which leads me to believe the kmalloc succeeded, however the data isn't displayed when I try to read from the 'data' variable again.
More confusingly, if I get rid of the kmalloc bit altogether, the data can be read from the driver. Although the problem then is it is followed by a load of other data because i don't have the opportunity to memset() it.
Am I using kmalloc correctly? Presumably not. How should I be doing this?
Additionally, my read function is as follows.
static ssize_t memo_read(struct file *f, char __user *buf,
size_t count, loff_t *f_pos){
ssize_t retval = 0;
printk("read function\n");
printk("data = %s\n", data);
if(*f_pos >= strlen(data)){
printk("EOF\n");
goto out;
}
if(copy_to_user(buf, data, strlen(data))){
retval = -EFAULT;
goto out;
}
printk("copy_to_user success\n");
*f_pos += strlen(data);
retval = strlen(data);
out:
return retval;
}
Thanks.
You should be using strlen_user()
on the userspace pointer, instead of strlen()
- and you should only call it once, and keep the result around (otherwise, you have a potential kernel exploit, because a second userspace thread could change the buffer while you're working on it).
Alternatively, you could use strncpy_from_user()
.
Apart from that, the kmalloc
looks OK.
(But really, as ephemient says, you should rethink your whole approach and use the count
argument instead of treating the input as a string).
Since you can't rely on the data written to a file being nul-terminated strings, you'll need to keep a data_len
length parameter around alongside the data
. Then your read
/write
implementations would be along these lines:
static char *data = NULL;
static size_t data_len;
static DEFINE_MUTEX(data_mutex);
static ssize_t memo_read(struct file *f, char __user *buf, size_t count, loff_t *f_pos
{
ssize_t retval = 0;
char *start;
mutex_lock(&data_mutex);
if (!data)
{
retval = -EINVAL; /* Or whatever you want to do here... */
goto out;
}
if (*f_pos >= data_len)
goto out; /* EOF */
start = data + *f_pos;
retval = data_len - *f_pos;
if (retval > count)
retval = count;
if (copy_to_user(buf, start, retval))
{
retval = -EFAULT;
goto out;
}
*f_pos += retval;
out:
mutex_unlock(&data_mutex);
return retval;
}
static ssize_t memo_write(struct file *filp, const char __user *buf, size_t count, loff_t *f_pos)
{
ssize_t retval = -ENOMEM;
mutex_lock(&data_mutex);
if (data)
kfree(data);
data = kmalloc(count, GFP_KERNEL);
if (!data)
goto out;
if (copy_from_user(data, buf, count))
{
kfree(data);
retval = -EFAULT;
goto out;
}
*f_pos = count;
retval = count;
data_len = count;
out:
mutex_unlock(&data_mutex);
return retval;
}
Don't forget to kfree(data)
in your error cases...
In any case, buf
is a pointer to user memory, so DON'T call strlen(buf)
. You must copy_from_user
first. Why not
data = kmalloc(count);
copy_from_user(data, buf, count);
?
Your read handler assumes that data
is a NUL-terminated string. When you were using an array, this may have been true by accident, but you never actually ensure this in your write handler. My guess is that copy_to_user
fails.
Here's a working example of a "memo" module that I wrote up just now, using kmalloc
:
#include <linux/fs.h>
#include <linux/miscdevice.h>
#include <linux/module.h>
#include <linux/types.h>
#include <linux/uaccess.h>
static char *data;
static size_t len;
static ssize_t
memo_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
{
ssize_t copy_len = min(len - min(len, *ppos), count);
ssize_t retval;
if (copy_to_user(buf, data + *ppos, copy_len)) {
retval = -EFAULT;
goto out;
}
*ppos += copy_len;
retval = copy_len;
out:
return retval;
}
static ssize_t
memo_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
{
ssize_t retval;
char *newdata;
newdata = kmalloc(count, GFP_KERNEL);
if (!newdata) {
retval = -ENOMEM;
goto out;
}
if (copy_from_user(newdata, buf, count)) {
retval = -EFAULT;
goto out;
}
kfree(data);
data = newdata;
newdata = NULL;
retval = len = count;
out:
kfree(newdata);
return retval;
}
static const struct file_operations memo_fops = {
.owner = THIS_MODULE,
.llseek = no_llseek,
.read = memo_read,
.write = memo_write,
};
static struct miscdevice memo_misc = { MISC_DYNAMIC_MINOR, "memo", &memo_fops };
static int __init memo_init(void)
{
int result;
result = misc_register(&memo_misc);
if (result < 0)
return -ENODEV;
return 0;
}
static void __exit memo_exit(void)
{
misc_deregister(&memo_misc);
kfree(data);
return;
}
module_init(memo_init);
module_exit(memo_exit);
MODULE_AUTHOR("ephemient");
MODULE_LICENSE("GPL");
Of course this is missing locking and other safety precautions, but I hope this helps.
精彩评论