Arjan: > On 10/3/2010 5:06 PM, Mai, Leonard wrote: >> + >> + thread = current_thread_info(); >> + comm = (char *)(struct task_struct *)(thread->task)->comm; > this is not only bad double casting (which seems buggy), it's also not doing the required locking. > get_task_comm() is the API to use here! >
Way back on this comment, 'get_task_comm()' cannot be used if the driver is to be a loadable module. There is actually a recent bug for nfsd.ko that ran into this too: http://www.spinics.net/lists/linux-nfs/msg16096.html There are many other drivers in the kernel just littered with just using 'current->comm, current-pid', etc instead of calling the function, which is actually what the fix was to the nfsd driver as well. I just did a cut-and-paste on get_task_comm() code found in fs/exec.c into the driver and it compiled cleanly (remains to be seen if it will work). How do you suggest solving this issue (since you will see it appear again in a review)? > (also, none of this is valid in irq context... I don't see you check for that) > > > >> + >> + mccontrol.channel = pti_control_channel; >> + pti_control_channel = (pti_control_channel + 1) & 0x7f; >> + >> + if (strlen(comm) < 23) >> + control_len = 9 + strlen(comm); > > > > > ... and what else if this is not true ? > + size_t size = USER_COPY_SIZE, n = 0; >> mc = filp->private_data; >> - kbuf = kmalloc(len, GFP_KERNEL); >> - if (kbuf == NULL) >> + kbuf = kmalloc(size, GFP_KERNEL); > > > 8Kb allocation? Really ? > > > _______________________________________________ Meego-kernel mailing list [email protected] http://lists.meego.com/listinfo/meego-kernel
