Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1289022

Re: [PATCH v2] staging: gdm72xx: add userspace data struct

From Wim de With <nauxuron@wimdewith.com>
Newsgroups linux.kernel
Subject Re: [PATCH v2] staging: gdm72xx: add userspace data struct
Date 2015-12-11 00:50 +0100
Message-ID <qEj8d-4Em-13@gated-at.bofh.it> (permalink)
References <qE7JN-5B8-27@gated-at.bofh.it> <qE7Tr-5EP-9@gated-at.bofh.it> <qEaHE-7v5-27@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


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.

> 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.

> 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.

> 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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH] staging: gdm72xx: add userspace data struct Wim de With <nauxuron@wimdewith.com> - 2015-12-10 10:20 +0100
  Re: [PATCH] staging: gdm72xx: add userspace data struct Dan Carpenter <dan.carpenter@oracle.com> - 2015-12-10 10:40 +0100
    Re: [PATCH] staging: gdm72xx: add userspace data struct Wim de With <nauxuron@wimdewith.com> - 2015-12-10 11:00 +0100
      Re: [PATCH] staging: gdm72xx: add userspace data struct Dan Carpenter <dan.carpenter@oracle.com> - 2015-12-10 12:40 +0100
        [PATCH v2] staging: gdm72xx: add userspace data struct Wim de With <nauxuron@wimdewith.com> - 2015-12-10 12:50 +0100
          Re: [PATCH v2] staging: gdm72xx: add userspace data struct One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk> - 2015-12-10 15:50 +0100
            Re: [PATCH v2] staging: gdm72xx: add userspace data struct Wim de With <nauxuron@wimdewith.com> - 2015-12-11 00:50 +0100
              Re: [PATCH v2] staging: gdm72xx: add userspace data struct One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk> - 2015-12-11 01:20 +0100
                [PATCH v3] staging: gdm72xx: add userspace data struct Wim de With <nauxuron@wimdewith.com> - 2015-12-11 10:30 +0100

csiph-web