On Fri, 11 Dec 2015 00:47:38 +0100 Wim de With <nauxuron@xxxxxxxxxxxxx> wrote: > On Thu, Dec 10, 2015 at 02:44:45PM +0000, One Thousand Gnomes wrote: > > (except that you mean sizeof(struct fsm_s) and it doesn't compile at the > > moment! > > Oops, sloppy mistake. Compile/test/send - even when in a hurry > > data_s can just be modified to be __user. All uses of it follow that > > rule. > > What do you mean? The data still needs to be copied from user space to kernel > space, if I'm not mistaken. And not all uses follow that rule, since in both > gdm_wimax_ioctl_get_data() and gdm_wimax_ioctl_set_data() it is used as both > the source and destination in the copy_from_user() and copy_to_user() call. Good point I missed that. > > All I think you need in this case is > > > > struct fsm_s fsm_buf; > > > > if (copy_from_user(&fsm_buf, req->data.buf,sizeof(buf)) > > return -EFAULT > > gdm_update_fsm(&fsm_buf); > > Do you mean sizeof(fsm_s)? I realize this would have been far simpler than my > overkill solution. Yes either sizeof(struct fsm_s) or sizeof(fsm_buf). The former is often safer. > > If you are touching the structs it might be wise to fix the other > > problems with them notably the use of int. sizes when used are unsigned - > > and signed sizes are asking for errors. In fact if you look at the > > existing uses of the size checks they look deeply suspicious the moment > > anything malicious passes in negative numbers. > > I would love to do that, but it is a bit outside the scope of this patch, so I > would rather safe this for another patch. Absolutely right - it should be another patch Alan _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
