lkml.org 
[lkml]   [2022]   [Nov]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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>

\
 
 \ /
  Last update: 2022-11-23 20:12    [W:3.013 / U:0.740 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site