search for: access_ok

...O ~/bin/make.cross chmod +x ~/bin/make.cross git checkout 51ce50c54895044f949129e595ed9a37e4d6c13a # save the attached .config to linux build tree make.cross ARCH=sparc64 All warnings (new ones prefixed by >>): drivers/vhost/vhost.c: In function 'umem_access_ok': >> drivers/vhost/vhost.c:915:30: warning: passing argument 2 of 'access_ok' makes pointer from integer without a cast [-Wint-conversion] !access_ok(VERIFY_READ, uaddr, size)) ^ In file included from arch/sparc/include/asm/uaccess.h:4:...

...O ~/bin/make.cross chmod +x ~/bin/make.cross git checkout 51ce50c54895044f949129e595ed9a37e4d6c13a # save the attached .config to linux build tree make.cross ARCH=sparc64 All warnings (new ones prefixed by >>): drivers/vhost/vhost.c: In function 'umem_access_ok': >> drivers/vhost/vhost.c:915:30: warning: passing argument 2 of 'access_ok' makes pointer from integer without a cast [-Wint-conversion] !access_ok(VERIFY_READ, uaddr, size)) ^ In file included from arch/sparc/include/asm/uaccess.h:4:...

...gt; + rsp.response = VIRTIO_SCSI_S_FUNCTION_REJECTED; > > > + resp = vq->iov[out].iov_base; > > > + ret = __copy_to_user(resp, &rsp, sizeof(rsp)); > > > > > > Is it actually safe to trust that iov_base has passed an earlier > > > access_ok() check here? Why not just use copy_to_user() instead? > > > > Good point. > > > > We really should have removed those double-underscore things ages ago. > > FWIW, on arm64 we always check/sanitize the user address as a result of > our sanitization of speculated...

...gt; + rsp.response = VIRTIO_SCSI_S_FUNCTION_REJECTED; > > > + resp = vq->iov[out].iov_base; > > > + ret = __copy_to_user(resp, &rsp, sizeof(rsp)); > > > > > > Is it actually safe to trust that iov_base has passed an earlier > > > access_ok() check here? Why not just use copy_to_user() instead? > > > > Good point. > > > > We really should have removed those double-underscore things ages ago. > > FWIW, on arm64 we always check/sanitize the user address as a result of > our sanitization of speculated...

...; > + memset(&rsp, 0, sizeof(rsp)); > + rsp.response = VIRTIO_SCSI_S_FUNCTION_REJECTED; > + resp = vq->iov[out].iov_base; > + ret = __copy_to_user(resp, &rsp, sizeof(rsp)); > > Is it actually safe to trust that iov_base has passed an earlier > access_ok() check here? Why not just use copy_to_user() instead? Good point. We really should have removed those double-underscore things ages ago. Also, apart from the address, what about the size? Wouldn't it be better to use copy_to_iter() rather than implement it badly by hand? Lin...

...; > + memset(&rsp, 0, sizeof(rsp)); > + rsp.response = VIRTIO_SCSI_S_FUNCTION_REJECTED; > + resp = vq->iov[out].iov_base; > + ret = __copy_to_user(resp, &rsp, sizeof(rsp)); > > Is it actually safe to trust that iov_base has passed an earlier > access_ok() check here? Why not just use copy_to_user() instead? Good point. We really should have removed those double-underscore things ages ago. Also, apart from the address, what about the size? Wouldn't it be better to use copy_to_iter() rather than implement it badly by hand? Lin...

If m.num is too large then the "m.num * sizeof(*m.arr)" multiplication could overflow and the access_ok() check wouldn't test the right size. Signed-off-by: Dan Carpenter <dan.carpenter at oracle.com> --- Only needed in linux-next. diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c index 215a3c0..fdff8f9 100644 --- a/drivers/xen/privcmd.c +++ b/drivers/xen/privcmd.c @@ -325,6 +325...

If m.num is too large then the "m.num * sizeof(*m.arr)" multiplication could overflow and the access_ok() check wouldn't test the right size. Signed-off-by: Dan Carpenter <dan.carpenter at oracle.com> --- Only needed in linux-next. diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c index 215a3c0..fdff8f9 100644 --- a/drivers/xen/privcmd.c +++ b/drivers/xen/privcmd.c @@ -325,6 +325...

On biarch kernels, it is not safe to do copy from/to user, even with use_mm, unless we checked the address range with access_ok previously. Implement these checks to enforce safe memory accesses. Reported-by: Al Viro <viro at zeniv.linux.org.uk> Signed-off-by: Michael S. Tsirkin <mst at redhat.com> --- drivers/vhost/net.c | 17 ++++++- drivers/vhost/vhost.c | 111 +++++++++++++++++++++++++++++++++++++++++...

On biarch kernels, it is not safe to do copy from/to user, even with use_mm, unless we checked the address range with access_ok previously. Implement these checks to enforce safe memory accesses. Reported-by: Al Viro <viro at zeniv.linux.org.uk> Signed-off-by: Michael S. Tsirkin <mst at redhat.com> --- drivers/vhost/net.c | 17 ++++++- drivers/vhost/vhost.c | 111 +++++++++++++++++++++++++++++++++++++++++...

...ers/vhost/vhost.h | 10 ++- > 2 files changed, 185 insertions(+), 16 deletions(-) > > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c > index 0f3f07c..cccbc82 100644 > --- a/drivers/vhost/vhost.c > +++ b/drivers/vhost/vhost.c > @@ -1115,10 +1115,15 @@ static int vq_access_ok_packed(struct vhost_virtqueue *vq, unsigned int num, > struct vring_used __user *used) > { > struct vring_desc_packed *packed = (struct vring_desc_packed *)desc; > + struct vring_packed_desc_event *driver_event = > + (struct vring_packed_desc_event *)avail; > + stru...

...ers/vhost/vhost.h | 10 ++- > 2 files changed, 185 insertions(+), 16 deletions(-) > > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c > index 0f3f07c..cccbc82 100644 > --- a/drivers/vhost/vhost.c > +++ b/drivers/vhost/vhost.c > @@ -1115,10 +1115,15 @@ static int vq_access_ok_packed(struct vhost_virtqueue *vq, unsigned int num, > struct vring_used __user *used) > { > struct vring_desc_packed *packed = (struct vring_desc_packed *)desc; > + struct vring_packed_desc_event *driver_event = > + (struct vring_packed_desc_event *)avail; > + stru...

On 2019/1/7 ??11:28, Michael S. Tsirkin wrote: > On Mon, Jan 07, 2019 at 10:19:03AM +0800, Jason Wang wrote: >> On 2019/1/3 ??4:47, Michael S. Tsirkin wrote: >>> On Sat, Dec 29, 2018 at 08:46:51PM +0800, Jason Wang wrote: >>>> This series tries to access virtqueue metadata through kernel virtual >>>> address instead of copy_user() friends since they had

On 2019/1/7 ??11:28, Michael S. Tsirkin wrote: > On Mon, Jan 07, 2019 at 10:19:03AM +0800, Jason Wang wrote: >> On 2019/1/3 ??4:47, Michael S. Tsirkin wrote: >>> On Sat, Dec 29, 2018 at 08:46:51PM +0800, Jason Wang wrote: >>>> This series tries to access virtqueue metadata through kernel virtual >>>> address instead of copy_user() friends since they had

On Fri, Nov 02, 2018 at 09:14:51AM -0700, Linus Torvalds wrote: > On Fri, Nov 2, 2018 at 6:04 AM Michael S. Tsirkin <mst at redhat.com> wrote: > > > > I've tried making access_ok mask the parameter it gets. > > PLEASE don't do this. Okay. > Just use "copy_to/from_user()". Just for completeness I'd like to point out for vhost the copies are done from the kernel thread. So yes we can switch to copy_to/from_user but for e.g. 32-bit userspace run...

On Fri, Nov 02, 2018 at 09:14:51AM -0700, Linus Torvalds wrote: > On Fri, Nov 2, 2018 at 6:04 AM Michael S. Tsirkin <mst at redhat.com> wrote: > > > > I've tried making access_ok mask the parameter it gets. > > PLEASE don't do this. Okay. > Just use "copy_to/from_user()". Just for completeness I'd like to point out for vhost the copies are done from the kernel thread. So yes we can switch to copy_to/from_user but for e.g. 32-bit userspace run...

I did an audit for potential integer overflows of values which get passed to access_ok() and here are the results. Signed-off-by: Dan Carpenter <error27 at gmail.com> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c index dd3d6f7..c2aa12c 100644 --- a/drivers/vhost/vhost.c +++ b/drivers/vhost/vhost.c @@ -429,6 +429,14 @@ static int vq_access_ok(unsigned int num,...

I did an audit for potential integer overflows of values which get passed to access_ok() and here are the results. Signed-off-by: Dan Carpenter <error27 at gmail.com> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c index dd3d6f7..c2aa12c 100644 --- a/drivers/vhost/vhost.c +++ b/drivers/vhost/vhost.c @@ -429,6 +429,14 @@ static int vq_access_ok(unsigned int num,...

On some architectures, the MMU can be disabled, leading to access_ok() becoming an empty macro that does not evaluate its size argument, which in turn produces an unused-variable warning: drivers/vhost/vhost.c:1191:9: error: unused variable 's' [-Werror,-Wunused-variable] size_t s = vhost_has_feature(vq, VIRTIO_RING_F_EVENT_IDX) ? 2 : 0; Mark the v...

...it to stable kernels (at least 3.10). > diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c > index 5445454..4be3832 100644 > --- a/fs/btrfs/send.c > +++ b/fs/btrfs/send.c > @@ -4553,8 +4553,8 @@ long btrfs_ioctl_send(struct file *mnt_file, void __user *arg_) > } > > if (!access_ok(VERIFY_READ, arg->clone_sources, > - sizeof(*arg->clone_sources * > - arg->clone_sources_count))) { > + sizeof(*arg->clone_sources) * > + arg->clone_sources_count)) { > ret = -EFAULT; > goto out; > } david -- To unsubscribe from this list: send t...