[PATCH] drm: remove redundant code form drm_ioc32.c
On 07/02/2015 11:10 AM, Jani Nikula wrote: > On Wed, 01 Jul 2015, Jarkko Sakkinen > wrote: >> The compat ioctl handler ends up calling access_ok() twice: first >> indirectly inside compat_alloc_user_space() and then after returning >> from that function. This patch fixes issue. >> >> Signed-off-by: Jarkko Sakkinen >> --- >> drivers/gpu/drm/drm_ioc32.c | 55 >> + >> 1 file changed, 26 insertions(+), 29 deletions(-) >> >> diff --git a/drivers/gpu/drm/drm_ioc32.c b/drivers/gpu/drm/drm_ioc32.c >> index aa8bbb4..77b63e7 100644 >> --- a/drivers/gpu/drm/drm_ioc32.c >> +++ b/drivers/gpu/drm/drm_ioc32.c >> @@ -93,7 +93,7 @@ static int compat_drm_version(struct file *file, unsigned >> int cmd, >> return -EFAULT; >> >> version = compat_alloc_user_space(sizeof(*version)); >> -if (!access_ok(VERIFY_WRITE, version, sizeof(*version))) >> +if (!version) >> return -EFAULT; >> if (__put_user(v32.name_len, >name_len) >> || __put_user((void __user *)(unsigned long)v32.name, >> @@ -140,7 +140,7 @@ static int compat_drm_getunique(struct file *file, >> unsigned int cmd, >> return -EFAULT; >> >> u = compat_alloc_user_space(sizeof(*u)); >> -if (!access_ok(VERIFY_WRITE, u, sizeof(*u))) >> +if (!u) >> return -EFAULT; >> if (__put_user(uq32.unique_len, >unique_len) >> || __put_user((void __user *)(unsigned long)uq32.unique, >> @@ -168,7 +168,7 @@ static int compat_drm_setunique(struct file *file, >> unsigned int cmd, >> return -EFAULT; >> >> u = compat_alloc_user_space(sizeof(*u)); >> -if (!access_ok(VERIFY_WRITE, u, sizeof(*u))) >> +if (!u) >> return -EFAULT; >> if (__put_user(uq32.unique_len, >unique_len) >> || __put_user((void __user *)(unsigned long)uq32.unique, >> @@ -200,7 +200,7 @@ static int compat_drm_getmap(struct file *file, unsigned >> int cmd, >> return -EFAULT; >> >> map = compat_alloc_user_space(sizeof(*map)); >> -if (!access_ok(VERIFY_WRITE, map, sizeof(*map))) >> +if (!map) >> return -EFAULT; >> if (__put_user(idx, >offset)) >> return -EFAULT; >> @@ -237,7 +237,7 @@ static int compat_drm_addmap(struct file *file, unsigned >> int cmd, >> return -EFAULT; >> >> map = compat_alloc_user_space(sizeof(*map)); >> -if (!access_ok(VERIFY_WRITE, map, sizeof(*map))) >> +if (!map) >> return -EFAULT; >> if (__put_user(m32.offset, >offset) >> || __put_user(m32.size, >size) >> @@ -277,7 +277,7 @@ static int compat_drm_rmmap(struct file *file, unsigned >> int cmd, >> return -EFAULT; >> >> map = compat_alloc_user_space(sizeof(*map)); >> -if (!access_ok(VERIFY_WRITE, map, sizeof(*map))) >> +if (!map) >> return -EFAULT; >> if (__put_user((void *)(unsigned long)handle, >handle)) >> return -EFAULT; >> @@ -306,7 +306,7 @@ static int compat_drm_getclient(struct file *file, >> unsigned int cmd, >> return -EFAULT; >> >> client = compat_alloc_user_space(sizeof(*client)); >> -if (!access_ok(VERIFY_WRITE, client, sizeof(*client))) >> +if (!client) >> return -EFAULT; >> if (__put_user(idx, >idx)) >> return -EFAULT; >> @@ -345,7 +345,7 @@ static int compat_drm_getstats(struct file *file, >> unsigned int cmd, >> int i, err; >> >> stats = compat_alloc_user_space(sizeof(*stats)); >> -if (!access_ok(VERIFY_WRITE, stats, sizeof(*stats))) >> +if (!stats) >> return -EFAULT; >> >> err = drm_ioctl(file, DRM_IOCTL_GET_STATS, (unsigned long)stats); >> @@ -382,8 +382,7 @@ static int compat_drm_addbufs(struct file *file, >> unsigned int cmd, >> unsigned long agp_start; >> >> buf = compat_alloc_user_space(sizeof(*buf)); >> -if (!access_ok(VERIFY_WRITE, buf, sizeof(*buf)) >> -|| !access_ok(VERIFY_WRITE, argp, sizeof(*argp))) >> +if (!buf || !argp) > The risks of touching old code... who does access_ok on argp now? That > doesn't come from compat_alloc_user_space. > > Same pattern repeated below a few times. Right. Thanks and sorry about sloppiness. I'll update the patch. > > BR, > Jani. > > >> return -EFAULT; >> >> if (__copy_in_user(buf, argp, offsetof(drm_buf_desc32_t, agp_start)) >> @@ -414,7 +413,7 @@ static int compat_drm_markbufs(struct file *file, >> unsigned int cmd, >> return -EFAULT; >> >> buf = compat_alloc_user_space(sizeof(*buf)); >> -if (!access_ok(VERIFY_WRITE, buf, sizeof(*buf))) >> +if (!buf) >> return -EFAULT; >> >> if (__put_user(b32.size, >size) >> @@ -455,7 +454,7 @@ static int compat_drm_infobufs(struct file *file, >> unsigned int cmd, >> >> nbytes = sizeof(*request) + count * sizeof(struct drm_buf_desc); >> request =
[PATCH] drm: remove redundant code form drm_ioc32.c
On 07/02/2015 10:32 AM, Daniel Vetter wrote: > On Wed, Jul 01, 2015 at 12:24:37PM +0300, Jarkko Sakkinen wrote: >> The compat ioctl handler ends up calling access_ok() twice: first >> indirectly inside compat_alloc_user_space() and then after returning >> from that function. This patch fixes issue. >> >> Signed-off-by: Jarkko Sakkinen > Applied to topic/drm-misc, thanks. Btw is this generated with some cocci > script or not? Nope. I just bumped into this when working on one yet-to-be-published driver on a completely different area and was comparing compat ioctl handlers. > -Daniel > >> --- >> drivers/gpu/drm/drm_ioc32.c | 55 >> + >> 1 file changed, 26 insertions(+), 29 deletions(-) >> >> diff --git a/drivers/gpu/drm/drm_ioc32.c b/drivers/gpu/drm/drm_ioc32.c >> index aa8bbb4..77b63e7 100644 >> --- a/drivers/gpu/drm/drm_ioc32.c >> +++ b/drivers/gpu/drm/drm_ioc32.c >> @@ -93,7 +93,7 @@ static int compat_drm_version(struct file *file, unsigned >> int cmd, >> return -EFAULT; >> >> version = compat_alloc_user_space(sizeof(*version)); >> -if (!access_ok(VERIFY_WRITE, version, sizeof(*version))) >> +if (!version) >> return -EFAULT; >> if (__put_user(v32.name_len, >name_len) >> || __put_user((void __user *)(unsigned long)v32.name, >> @@ -140,7 +140,7 @@ static int compat_drm_getunique(struct file *file, >> unsigned int cmd, >> return -EFAULT; >> >> u = compat_alloc_user_space(sizeof(*u)); >> -if (!access_ok(VERIFY_WRITE, u, sizeof(*u))) >> +if (!u) >> return -EFAULT; >> if (__put_user(uq32.unique_len, >unique_len) >> || __put_user((void __user *)(unsigned long)uq32.unique, >> @@ -168,7 +168,7 @@ static int compat_drm_setunique(struct file *file, >> unsigned int cmd, >> return -EFAULT; >> >> u = compat_alloc_user_space(sizeof(*u)); >> -if (!access_ok(VERIFY_WRITE, u, sizeof(*u))) >> +if (!u) >> return -EFAULT; >> if (__put_user(uq32.unique_len, >unique_len) >> || __put_user((void __user *)(unsigned long)uq32.unique, >> @@ -200,7 +200,7 @@ static int compat_drm_getmap(struct file *file, unsigned >> int cmd, >> return -EFAULT; >> >> map = compat_alloc_user_space(sizeof(*map)); >> -if (!access_ok(VERIFY_WRITE, map, sizeof(*map))) >> +if (!map) >> return -EFAULT; >> if (__put_user(idx, >offset)) >> return -EFAULT; >> @@ -237,7 +237,7 @@ static int compat_drm_addmap(struct file *file, unsigned >> int cmd, >> return -EFAULT; >> >> map = compat_alloc_user_space(sizeof(*map)); >> -if (!access_ok(VERIFY_WRITE, map, sizeof(*map))) >> +if (!map) >> return -EFAULT; >> if (__put_user(m32.offset, >offset) >> || __put_user(m32.size, >size) >> @@ -277,7 +277,7 @@ static int compat_drm_rmmap(struct file *file, unsigned >> int cmd, >> return -EFAULT; >> >> map = compat_alloc_user_space(sizeof(*map)); >> -if (!access_ok(VERIFY_WRITE, map, sizeof(*map))) >> +if (!map) >> return -EFAULT; >> if (__put_user((void *)(unsigned long)handle, >handle)) >> return -EFAULT; >> @@ -306,7 +306,7 @@ static int compat_drm_getclient(struct file *file, >> unsigned int cmd, >> return -EFAULT; >> >> client = compat_alloc_user_space(sizeof(*client)); >> -if (!access_ok(VERIFY_WRITE, client, sizeof(*client))) >> +if (!client) >> return -EFAULT; >> if (__put_user(idx, >idx)) >> return -EFAULT; >> @@ -345,7 +345,7 @@ static int compat_drm_getstats(struct file *file, >> unsigned int cmd, >> int i, err; >> >> stats = compat_alloc_user_space(sizeof(*stats)); >> -if (!access_ok(VERIFY_WRITE, stats, sizeof(*stats))) >> +if (!stats) >> return -EFAULT; >> >> err = drm_ioctl(file, DRM_IOCTL_GET_STATS, (unsigned long)stats); >> @@ -382,8 +382,7 @@ static int compat_drm_addbufs(struct file *file, >> unsigned int cmd, >> unsigned long agp_start; >> >> buf = compat_alloc_user_space(sizeof(*buf)); >> -if (!access_ok(VERIFY_WRITE, buf, sizeof(*buf)) >> -|| !access_ok(VERIFY_WRITE, argp, sizeof(*argp))) >> +if (!buf || !argp) >> return -EFAULT; >> >> if (__copy_in_user(buf, argp, offsetof(drm_buf_desc32_t, agp_start)) >> @@ -414,7 +413,7 @@ static int compat_drm_markbufs(struct file *file, >> unsigned int cmd, >> return -EFAULT; >> >> buf = compat_alloc_user_space(sizeof(*buf)); >> -if (!access_ok(VERIFY_WRITE, buf, sizeof(*buf))) >> +if (!buf) >> return -EFAULT; >> >> if (__put_user(b32.size, >size) >> @@ -455,7 +454,7 @@ static int compat_drm_infobufs(struct file *file, >> unsigned int cmd, >> >> nbytes = sizeof(*request) + count * sizeof(struct drm_buf_desc); >>
[PATCH] drm: remove redundant code form drm_ioc32.c
On Thu, Jul 02, 2015 at 11:10:06AM +0300, Jani Nikula wrote: > On Wed, 01 Jul 2015, Jarkko Sakkinen > wrote: > > The compat ioctl handler ends up calling access_ok() twice: first > > indirectly inside compat_alloc_user_space() and then after returning > > from that function. This patch fixes issue. > > > > Signed-off-by: Jarkko Sakkinen > > --- > > drivers/gpu/drm/drm_ioc32.c | 55 > > + > > 1 file changed, 26 insertions(+), 29 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_ioc32.c b/drivers/gpu/drm/drm_ioc32.c > > index aa8bbb4..77b63e7 100644 > > --- a/drivers/gpu/drm/drm_ioc32.c > > +++ b/drivers/gpu/drm/drm_ioc32.c > > @@ -93,7 +93,7 @@ static int compat_drm_version(struct file *file, unsigned > > int cmd, > > return -EFAULT; > > > > version = compat_alloc_user_space(sizeof(*version)); > > - if (!access_ok(VERIFY_WRITE, version, sizeof(*version))) > > + if (!version) > > return -EFAULT; > > if (__put_user(v32.name_len, >name_len) > > || __put_user((void __user *)(unsigned long)v32.name, > > @@ -140,7 +140,7 @@ static int compat_drm_getunique(struct file *file, > > unsigned int cmd, > > return -EFAULT; > > > > u = compat_alloc_user_space(sizeof(*u)); > > - if (!access_ok(VERIFY_WRITE, u, sizeof(*u))) > > + if (!u) > > return -EFAULT; > > if (__put_user(uq32.unique_len, >unique_len) > > || __put_user((void __user *)(unsigned long)uq32.unique, > > @@ -168,7 +168,7 @@ static int compat_drm_setunique(struct file *file, > > unsigned int cmd, > > return -EFAULT; > > > > u = compat_alloc_user_space(sizeof(*u)); > > - if (!access_ok(VERIFY_WRITE, u, sizeof(*u))) > > + if (!u) > > return -EFAULT; > > if (__put_user(uq32.unique_len, >unique_len) > > || __put_user((void __user *)(unsigned long)uq32.unique, > > @@ -200,7 +200,7 @@ static int compat_drm_getmap(struct file *file, > > unsigned int cmd, > > return -EFAULT; > > > > map = compat_alloc_user_space(sizeof(*map)); > > - if (!access_ok(VERIFY_WRITE, map, sizeof(*map))) > > + if (!map) > > return -EFAULT; > > if (__put_user(idx, >offset)) > > return -EFAULT; > > @@ -237,7 +237,7 @@ static int compat_drm_addmap(struct file *file, > > unsigned int cmd, > > return -EFAULT; > > > > map = compat_alloc_user_space(sizeof(*map)); > > - if (!access_ok(VERIFY_WRITE, map, sizeof(*map))) > > + if (!map) > > return -EFAULT; > > if (__put_user(m32.offset, >offset) > > || __put_user(m32.size, >size) > > @@ -277,7 +277,7 @@ static int compat_drm_rmmap(struct file *file, unsigned > > int cmd, > > return -EFAULT; > > > > map = compat_alloc_user_space(sizeof(*map)); > > - if (!access_ok(VERIFY_WRITE, map, sizeof(*map))) > > + if (!map) > > return -EFAULT; > > if (__put_user((void *)(unsigned long)handle, >handle)) > > return -EFAULT; > > @@ -306,7 +306,7 @@ static int compat_drm_getclient(struct file *file, > > unsigned int cmd, > > return -EFAULT; > > > > client = compat_alloc_user_space(sizeof(*client)); > > - if (!access_ok(VERIFY_WRITE, client, sizeof(*client))) > > + if (!client) > > return -EFAULT; > > if (__put_user(idx, >idx)) > > return -EFAULT; > > @@ -345,7 +345,7 @@ static int compat_drm_getstats(struct file *file, > > unsigned int cmd, > > int i, err; > > > > stats = compat_alloc_user_space(sizeof(*stats)); > > - if (!access_ok(VERIFY_WRITE, stats, sizeof(*stats))) > > + if (!stats) > > return -EFAULT; > > > > err = drm_ioctl(file, DRM_IOCTL_GET_STATS, (unsigned long)stats); > > @@ -382,8 +382,7 @@ static int compat_drm_addbufs(struct file *file, > > unsigned int cmd, > > unsigned long agp_start; > > > > buf = compat_alloc_user_space(sizeof(*buf)); > > - if (!access_ok(VERIFY_WRITE, buf, sizeof(*buf)) > > - || !access_ok(VERIFY_WRITE, argp, sizeof(*argp))) > > + if (!buf || !argp) > > The risks of touching old code... who does access_ok on argp now? That > doesn't come from compat_alloc_user_space. > > Same pattern repeated below a few times. Oops, good catch. Patch dropped again. -Daniel > > BR, > Jani. > > > > return -EFAULT; > > > > if (__copy_in_user(buf, argp, offsetof(drm_buf_desc32_t, agp_start)) > > @@ -414,7 +413,7 @@ static int compat_drm_markbufs(struct file *file, > > unsigned int cmd, > > return -EFAULT; > > > > buf = compat_alloc_user_space(sizeof(*buf)); > > - if (!access_ok(VERIFY_WRITE, buf, sizeof(*buf))) > > + if (!buf) > > return -EFAULT; > > > > if (__put_user(b32.size, >size) > > @@ -455,7 +454,7 @@ static int compat_drm_infobufs(struct file *file, > > unsigned int cmd, > > > > nbytes = sizeof(*request) + count * sizeof(struct
[PATCH] drm: remove redundant code form drm_ioc32.c
On Wed, 01 Jul 2015, Jarkko Sakkinen wrote: > The compat ioctl handler ends up calling access_ok() twice: first > indirectly inside compat_alloc_user_space() and then after returning > from that function. This patch fixes issue. > > Signed-off-by: Jarkko Sakkinen > --- > drivers/gpu/drm/drm_ioc32.c | 55 > + > 1 file changed, 26 insertions(+), 29 deletions(-) > > diff --git a/drivers/gpu/drm/drm_ioc32.c b/drivers/gpu/drm/drm_ioc32.c > index aa8bbb4..77b63e7 100644 > --- a/drivers/gpu/drm/drm_ioc32.c > +++ b/drivers/gpu/drm/drm_ioc32.c > @@ -93,7 +93,7 @@ static int compat_drm_version(struct file *file, unsigned > int cmd, > return -EFAULT; > > version = compat_alloc_user_space(sizeof(*version)); > - if (!access_ok(VERIFY_WRITE, version, sizeof(*version))) > + if (!version) > return -EFAULT; > if (__put_user(v32.name_len, >name_len) > || __put_user((void __user *)(unsigned long)v32.name, > @@ -140,7 +140,7 @@ static int compat_drm_getunique(struct file *file, > unsigned int cmd, > return -EFAULT; > > u = compat_alloc_user_space(sizeof(*u)); > - if (!access_ok(VERIFY_WRITE, u, sizeof(*u))) > + if (!u) > return -EFAULT; > if (__put_user(uq32.unique_len, >unique_len) > || __put_user((void __user *)(unsigned long)uq32.unique, > @@ -168,7 +168,7 @@ static int compat_drm_setunique(struct file *file, > unsigned int cmd, > return -EFAULT; > > u = compat_alloc_user_space(sizeof(*u)); > - if (!access_ok(VERIFY_WRITE, u, sizeof(*u))) > + if (!u) > return -EFAULT; > if (__put_user(uq32.unique_len, >unique_len) > || __put_user((void __user *)(unsigned long)uq32.unique, > @@ -200,7 +200,7 @@ static int compat_drm_getmap(struct file *file, unsigned > int cmd, > return -EFAULT; > > map = compat_alloc_user_space(sizeof(*map)); > - if (!access_ok(VERIFY_WRITE, map, sizeof(*map))) > + if (!map) > return -EFAULT; > if (__put_user(idx, >offset)) > return -EFAULT; > @@ -237,7 +237,7 @@ static int compat_drm_addmap(struct file *file, unsigned > int cmd, > return -EFAULT; > > map = compat_alloc_user_space(sizeof(*map)); > - if (!access_ok(VERIFY_WRITE, map, sizeof(*map))) > + if (!map) > return -EFAULT; > if (__put_user(m32.offset, >offset) > || __put_user(m32.size, >size) > @@ -277,7 +277,7 @@ static int compat_drm_rmmap(struct file *file, unsigned > int cmd, > return -EFAULT; > > map = compat_alloc_user_space(sizeof(*map)); > - if (!access_ok(VERIFY_WRITE, map, sizeof(*map))) > + if (!map) > return -EFAULT; > if (__put_user((void *)(unsigned long)handle, >handle)) > return -EFAULT; > @@ -306,7 +306,7 @@ static int compat_drm_getclient(struct file *file, > unsigned int cmd, > return -EFAULT; > > client = compat_alloc_user_space(sizeof(*client)); > - if (!access_ok(VERIFY_WRITE, client, sizeof(*client))) > + if (!client) > return -EFAULT; > if (__put_user(idx, >idx)) > return -EFAULT; > @@ -345,7 +345,7 @@ static int compat_drm_getstats(struct file *file, > unsigned int cmd, > int i, err; > > stats = compat_alloc_user_space(sizeof(*stats)); > - if (!access_ok(VERIFY_WRITE, stats, sizeof(*stats))) > + if (!stats) > return -EFAULT; > > err = drm_ioctl(file, DRM_IOCTL_GET_STATS, (unsigned long)stats); > @@ -382,8 +382,7 @@ static int compat_drm_addbufs(struct file *file, unsigned > int cmd, > unsigned long agp_start; > > buf = compat_alloc_user_space(sizeof(*buf)); > - if (!access_ok(VERIFY_WRITE, buf, sizeof(*buf)) > - || !access_ok(VERIFY_WRITE, argp, sizeof(*argp))) > + if (!buf || !argp) The risks of touching old code... who does access_ok on argp now? That doesn't come from compat_alloc_user_space. Same pattern repeated below a few times. BR, Jani. > return -EFAULT; > > if (__copy_in_user(buf, argp, offsetof(drm_buf_desc32_t, agp_start)) > @@ -414,7 +413,7 @@ static int compat_drm_markbufs(struct file *file, > unsigned int cmd, > return -EFAULT; > > buf = compat_alloc_user_space(sizeof(*buf)); > - if (!access_ok(VERIFY_WRITE, buf, sizeof(*buf))) > + if (!buf) > return -EFAULT; > > if (__put_user(b32.size, >size) > @@ -455,7 +454,7 @@ static int compat_drm_infobufs(struct file *file, > unsigned int cmd, > > nbytes = sizeof(*request) + count * sizeof(struct drm_buf_desc); > request = compat_alloc_user_space(nbytes); > - if (!access_ok(VERIFY_WRITE, request, nbytes)) > + if (!request) > return -EFAULT; > list = (struct drm_buf_desc *) (request + 1); > > @@
[PATCH] drm: remove redundant code form drm_ioc32.c
On Wed, Jul 01, 2015 at 12:24:37PM +0300, Jarkko Sakkinen wrote: > The compat ioctl handler ends up calling access_ok() twice: first > indirectly inside compat_alloc_user_space() and then after returning > from that function. This patch fixes issue. > > Signed-off-by: Jarkko Sakkinen Applied to topic/drm-misc, thanks. Btw is this generated with some cocci script or not? -Daniel > --- > drivers/gpu/drm/drm_ioc32.c | 55 > + > 1 file changed, 26 insertions(+), 29 deletions(-) > > diff --git a/drivers/gpu/drm/drm_ioc32.c b/drivers/gpu/drm/drm_ioc32.c > index aa8bbb4..77b63e7 100644 > --- a/drivers/gpu/drm/drm_ioc32.c > +++ b/drivers/gpu/drm/drm_ioc32.c > @@ -93,7 +93,7 @@ static int compat_drm_version(struct file *file, unsigned > int cmd, > return -EFAULT; > > version = compat_alloc_user_space(sizeof(*version)); > - if (!access_ok(VERIFY_WRITE, version, sizeof(*version))) > + if (!version) > return -EFAULT; > if (__put_user(v32.name_len, >name_len) > || __put_user((void __user *)(unsigned long)v32.name, > @@ -140,7 +140,7 @@ static int compat_drm_getunique(struct file *file, > unsigned int cmd, > return -EFAULT; > > u = compat_alloc_user_space(sizeof(*u)); > - if (!access_ok(VERIFY_WRITE, u, sizeof(*u))) > + if (!u) > return -EFAULT; > if (__put_user(uq32.unique_len, >unique_len) > || __put_user((void __user *)(unsigned long)uq32.unique, > @@ -168,7 +168,7 @@ static int compat_drm_setunique(struct file *file, > unsigned int cmd, > return -EFAULT; > > u = compat_alloc_user_space(sizeof(*u)); > - if (!access_ok(VERIFY_WRITE, u, sizeof(*u))) > + if (!u) > return -EFAULT; > if (__put_user(uq32.unique_len, >unique_len) > || __put_user((void __user *)(unsigned long)uq32.unique, > @@ -200,7 +200,7 @@ static int compat_drm_getmap(struct file *file, unsigned > int cmd, > return -EFAULT; > > map = compat_alloc_user_space(sizeof(*map)); > - if (!access_ok(VERIFY_WRITE, map, sizeof(*map))) > + if (!map) > return -EFAULT; > if (__put_user(idx, >offset)) > return -EFAULT; > @@ -237,7 +237,7 @@ static int compat_drm_addmap(struct file *file, unsigned > int cmd, > return -EFAULT; > > map = compat_alloc_user_space(sizeof(*map)); > - if (!access_ok(VERIFY_WRITE, map, sizeof(*map))) > + if (!map) > return -EFAULT; > if (__put_user(m32.offset, >offset) > || __put_user(m32.size, >size) > @@ -277,7 +277,7 @@ static int compat_drm_rmmap(struct file *file, unsigned > int cmd, > return -EFAULT; > > map = compat_alloc_user_space(sizeof(*map)); > - if (!access_ok(VERIFY_WRITE, map, sizeof(*map))) > + if (!map) > return -EFAULT; > if (__put_user((void *)(unsigned long)handle, >handle)) > return -EFAULT; > @@ -306,7 +306,7 @@ static int compat_drm_getclient(struct file *file, > unsigned int cmd, > return -EFAULT; > > client = compat_alloc_user_space(sizeof(*client)); > - if (!access_ok(VERIFY_WRITE, client, sizeof(*client))) > + if (!client) > return -EFAULT; > if (__put_user(idx, >idx)) > return -EFAULT; > @@ -345,7 +345,7 @@ static int compat_drm_getstats(struct file *file, > unsigned int cmd, > int i, err; > > stats = compat_alloc_user_space(sizeof(*stats)); > - if (!access_ok(VERIFY_WRITE, stats, sizeof(*stats))) > + if (!stats) > return -EFAULT; > > err = drm_ioctl(file, DRM_IOCTL_GET_STATS, (unsigned long)stats); > @@ -382,8 +382,7 @@ static int compat_drm_addbufs(struct file *file, unsigned > int cmd, > unsigned long agp_start; > > buf = compat_alloc_user_space(sizeof(*buf)); > - if (!access_ok(VERIFY_WRITE, buf, sizeof(*buf)) > - || !access_ok(VERIFY_WRITE, argp, sizeof(*argp))) > + if (!buf || !argp) > return -EFAULT; > > if (__copy_in_user(buf, argp, offsetof(drm_buf_desc32_t, agp_start)) > @@ -414,7 +413,7 @@ static int compat_drm_markbufs(struct file *file, > unsigned int cmd, > return -EFAULT; > > buf = compat_alloc_user_space(sizeof(*buf)); > - if (!access_ok(VERIFY_WRITE, buf, sizeof(*buf))) > + if (!buf) > return -EFAULT; > > if (__put_user(b32.size, >size) > @@ -455,7 +454,7 @@ static int compat_drm_infobufs(struct file *file, > unsigned int cmd, > > nbytes = sizeof(*request) + count * sizeof(struct drm_buf_desc); > request = compat_alloc_user_space(nbytes); > - if (!access_ok(VERIFY_WRITE, request, nbytes)) > + if (!request) > return -EFAULT; > list = (struct drm_buf_desc *) (request + 1); > > @@ -516,7 +515,7 @@ static int
[PATCH] drm: remove redundant code form drm_ioc32.c
The compat ioctl handler ends up calling access_ok() twice: first indirectly inside compat_alloc_user_space() and then after returning from that function. This patch fixes issue. Signed-off-by: Jarkko Sakkinen --- drivers/gpu/drm/drm_ioc32.c | 55 + 1 file changed, 26 insertions(+), 29 deletions(-) diff --git a/drivers/gpu/drm/drm_ioc32.c b/drivers/gpu/drm/drm_ioc32.c index aa8bbb4..77b63e7 100644 --- a/drivers/gpu/drm/drm_ioc32.c +++ b/drivers/gpu/drm/drm_ioc32.c @@ -93,7 +93,7 @@ static int compat_drm_version(struct file *file, unsigned int cmd, return -EFAULT; version = compat_alloc_user_space(sizeof(*version)); - if (!access_ok(VERIFY_WRITE, version, sizeof(*version))) + if (!version) return -EFAULT; if (__put_user(v32.name_len, >name_len) || __put_user((void __user *)(unsigned long)v32.name, @@ -140,7 +140,7 @@ static int compat_drm_getunique(struct file *file, unsigned int cmd, return -EFAULT; u = compat_alloc_user_space(sizeof(*u)); - if (!access_ok(VERIFY_WRITE, u, sizeof(*u))) + if (!u) return -EFAULT; if (__put_user(uq32.unique_len, >unique_len) || __put_user((void __user *)(unsigned long)uq32.unique, @@ -168,7 +168,7 @@ static int compat_drm_setunique(struct file *file, unsigned int cmd, return -EFAULT; u = compat_alloc_user_space(sizeof(*u)); - if (!access_ok(VERIFY_WRITE, u, sizeof(*u))) + if (!u) return -EFAULT; if (__put_user(uq32.unique_len, >unique_len) || __put_user((void __user *)(unsigned long)uq32.unique, @@ -200,7 +200,7 @@ static int compat_drm_getmap(struct file *file, unsigned int cmd, return -EFAULT; map = compat_alloc_user_space(sizeof(*map)); - if (!access_ok(VERIFY_WRITE, map, sizeof(*map))) + if (!map) return -EFAULT; if (__put_user(idx, >offset)) return -EFAULT; @@ -237,7 +237,7 @@ static int compat_drm_addmap(struct file *file, unsigned int cmd, return -EFAULT; map = compat_alloc_user_space(sizeof(*map)); - if (!access_ok(VERIFY_WRITE, map, sizeof(*map))) + if (!map) return -EFAULT; if (__put_user(m32.offset, >offset) || __put_user(m32.size, >size) @@ -277,7 +277,7 @@ static int compat_drm_rmmap(struct file *file, unsigned int cmd, return -EFAULT; map = compat_alloc_user_space(sizeof(*map)); - if (!access_ok(VERIFY_WRITE, map, sizeof(*map))) + if (!map) return -EFAULT; if (__put_user((void *)(unsigned long)handle, >handle)) return -EFAULT; @@ -306,7 +306,7 @@ static int compat_drm_getclient(struct file *file, unsigned int cmd, return -EFAULT; client = compat_alloc_user_space(sizeof(*client)); - if (!access_ok(VERIFY_WRITE, client, sizeof(*client))) + if (!client) return -EFAULT; if (__put_user(idx, >idx)) return -EFAULT; @@ -345,7 +345,7 @@ static int compat_drm_getstats(struct file *file, unsigned int cmd, int i, err; stats = compat_alloc_user_space(sizeof(*stats)); - if (!access_ok(VERIFY_WRITE, stats, sizeof(*stats))) + if (!stats) return -EFAULT; err = drm_ioctl(file, DRM_IOCTL_GET_STATS, (unsigned long)stats); @@ -382,8 +382,7 @@ static int compat_drm_addbufs(struct file *file, unsigned int cmd, unsigned long agp_start; buf = compat_alloc_user_space(sizeof(*buf)); - if (!access_ok(VERIFY_WRITE, buf, sizeof(*buf)) - || !access_ok(VERIFY_WRITE, argp, sizeof(*argp))) + if (!buf || !argp) return -EFAULT; if (__copy_in_user(buf, argp, offsetof(drm_buf_desc32_t, agp_start)) @@ -414,7 +413,7 @@ static int compat_drm_markbufs(struct file *file, unsigned int cmd, return -EFAULT; buf = compat_alloc_user_space(sizeof(*buf)); - if (!access_ok(VERIFY_WRITE, buf, sizeof(*buf))) + if (!buf) return -EFAULT; if (__put_user(b32.size, >size) @@ -455,7 +454,7 @@ static int compat_drm_infobufs(struct file *file, unsigned int cmd, nbytes = sizeof(*request) + count * sizeof(struct drm_buf_desc); request = compat_alloc_user_space(nbytes); - if (!access_ok(VERIFY_WRITE, request, nbytes)) + if (!request) return -EFAULT; list = (struct drm_buf_desc *) (request + 1); @@ -516,7 +515,7 @@ static int compat_drm_mapbufs(struct file *file, unsigned int cmd, return -EINVAL; nbytes = sizeof(*request) + count * sizeof(struct drm_buf_pub); request = compat_alloc_user_space(nbytes); - if (!access_ok(VERIFY_WRITE, request, nbytes)) + if (!request)
