Messages in this thread | | | Date | Wed, 23 Nov 2022 19:12:03 +0000 | From | Carlos Llamas <> | Subject | Re: [PATCH] binder: remove unneeded size check code |
| |
On Tue, Nov 15, 2022 at 08:03:51PM +0800, Jiazi.Li wrote: > In binder_ioctl function, the legitimacy check of cmd size has been > done in switch-case code: > switch (cmd) { > case BINDER_WRITE_READ;//BINDER_WRITE_READ contains size info > > So unneeded do size check in binder_ioctl and binder_ioctl_write_read > again. > > In the following version of Google GKI: > > Linux version 5.10.110-android12-9-00011-g2c814f559132-ab8969555 > > It seems that the compiler has made optimization and has not passed > cmd parameters to binder_ioctl_write_read: > <binder_ioctl+628>: mov w8, #0x6201 // #25089 > <binder_ioctl+632>: movk w8, #0xc030, lsl #16 > <binder_ioctl+636>: cmp w20, w8 > <binder_ioctl+640>: b.ne 0xffffffda8aa97880 <binder_ioctl+3168> > <binder_ioctl+644>: mov x0, x23 //filp > <binder_ioctl+648>: mov x1, x27 //arg > <binder_ioctl+652>: mov x2, x22 //thread > <binder_ioctl+656>: bl 0xffffffda8aa9e6e4 <binder_ioctl_write_read> > <binder_ioctl+660>: mov w26, w0 > > Signed-off-by: Jiazi.Li <jiazi.li@transsion.com> > --- > drivers/android/binder.c | 15 ++------------- > 1 file changed, 2 insertions(+), 13 deletions(-) > > diff --git a/drivers/android/binder.c b/drivers/android/binder.c > index 880224ec6abb..48e5a3531282 100644 > --- a/drivers/android/binder.c > +++ b/drivers/android/binder.c > @@ -5006,20 +5006,14 @@ static __poll_t binder_poll(struct file *filp, > return 0; > } > > -static int binder_ioctl_write_read(struct file *filp, > - unsigned int cmd, unsigned long arg, > +static int binder_ioctl_write_read(struct file *filp, unsigned long arg, > struct binder_thread *thread) > { > int ret = 0; > struct binder_proc *proc = filp->private_data; > - unsigned int size = _IOC_SIZE(cmd); > void __user *ubuf = (void __user *)arg; > struct binder_write_read bwr; > > - if (size != sizeof(struct binder_write_read)) { > - ret = -EINVAL; > - goto out; > - } > if (copy_from_user(&bwr, ubuf, sizeof(bwr))) { > ret = -EFAULT; > goto out; > @@ -5296,7 +5290,6 @@ static long binder_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) > int ret; > struct binder_proc *proc = filp->private_data; > struct binder_thread *thread; > - unsigned int size = _IOC_SIZE(cmd); > void __user *ubuf = (void __user *)arg; > > /*pr_info("binder_ioctl: %d:%d %x %lx\n", > @@ -5318,7 +5311,7 @@ static long binder_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) > > switch (cmd) { > case BINDER_WRITE_READ: > - ret = binder_ioctl_write_read(filp, cmd, arg, thread); > + ret = binder_ioctl_write_read(filp, arg, thread); > if (ret) > goto err; > break; > @@ -5361,10 +5354,6 @@ static long binder_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) > case BINDER_VERSION: { > struct binder_version __user *ver = ubuf; > > - if (size != sizeof(struct binder_version)) { > - ret = -EINVAL; > - goto err; > - } > if (put_user(BINDER_CURRENT_PROTOCOL_VERSION, > &ver->protocol_version)) { > ret = -EINVAL; > -- > 2.17.1 >
Looks good, thanks!
Acked-by: Carlos Llamas <cmllamas@google.com>
| |