Messages in this thread | | | From | Andries.Brouwer@cwi ... | Date | Mon, 29 Sep 2003 20:25:11 +0200 (MEST) | Subject | Re: [PATCH] autofs sparse fixes |
| |
aeb:
/* Our write does not write to user space */ write = (ssize_t (*)(struct file *, const char *, size_t, loff_t *)) file->f_op->write;
lbt:
For cases like this, where we use a kernel pointer and do the "set_fs(KERNEL_DS)" thing to make a user-pointer routine write to kernel space, I suggest casting the pointer instead of the function, along with a comment on the "set_fs()".
set_fs(KERNEL_DS);
/* This cast is legal due to the set_fs()! */ data = (const void __user *) addr;
...->write(file, data, bytes); set_fs(old_fs);
See? That makes the cast more obvious, and it also makes it obvious _why_ the cast from kernel->user pointer is ok in this case.
Hmm. Yes. Hmm. I have to admit that my cast is complicated, one might even say messy, but as always - when something seems messy it can be made to look less so by using more lines of code. E.g.,
write = (write_to_kernel_t) file->f_op->write;
looks less intimidating, and write_to_kernel_t can be defined next to write_proc_t that we have already.
I think I prefer two rights above two wrongs - the declaration already gives addr the right type - it really does point to kernel space - but we inherit an incorrect type for file->f_op->write, so have to cast that.
Hmm. In reality maybe I have no strong feelings either way. Will see what you do and possibly send some other version of what you did not apply.
Andries
[Soon things get more interesting. These were the trivial cases, with set_fs nearby and to a constant value. In other words, cases where we know precisely what happens and only have to add a cast. Intermezzo has a lot of code where the same code may be executed with kernel and with user pointers. Sendfile can use kernel or user pointers. It is declared with __user so requires
retval = in_file->f_op->sendfile(in_file, ppos, count, file_send_actor, (void __user *) out_file);
(fs/read_write.c:585 -- two wrongs indeed) where the type of pointer is implicit in the actor. Struct tty_operations has the field int (*write)(struct tty_struct *tty, int from_user, const unsigned char *buf, int count); Etc. Lots of places where static markup does not suffice to show which pointers point to user space. Pity. That diminishes the value of __user markup greatly.] - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |