Page 1 of 2

I2C dev interface

PostPosted: Wed Jul 30, 2014 2:30 pm
by gregkmoff
Has anyone done much with the I2C dev interface under GRSEC? That's the interface where an I2C bus shows up as a /dev entry.

Specifically, I'm having trouble doing combined read/writes from userspace. My app does an ioctl(fd, I2C_RDWR, &data) but I always seem to get a failure when the kernel is doing a memdup_user()...

The userspace code is legacy and has always worked, until we migrated to GRSEC.

We're running with the 3.14.8 GRSEC patch.

Re: I2C dev interface

PostPosted: Wed Jul 30, 2014 3:57 pm
by PaX Team
1. can you log the arguments of the failing memdup_user?
2. can you post the grsec bits of the kernel config?

Re: I2C dev interface

PostPosted: Wed Jul 30, 2014 4:10 pm
by gregkmoff
The code that fails is in the kernel

file: drivers/i2c/i2c-dev.c
this block:

Code: Select all
                data_ptrs[i] = (u8 __force_user *)rdwr_pa[i].buf;
                rdwr_pa[i].buf = memdup_user(data_ptrs[i], rdwr_pa[i].len);
                if (IS_ERR(rdwr_pa[i].buf)) {
                        res = PTR_ERR(rdwr_pa[i].buf);
                        break;
                }



Relevant kernel config is as follows:

CONFIG_GRKERNSEC=y
# CONFIG_GRKERNSEC_CONFIG_AUTO is not set
CONFIG_GRKERNSEC_CONFIG_CUSTOM=y
CONFIG_GRKERNSEC_KMEM=y
CONFIG_GRKERNSEC_PERF_HARDEN=y
# CONFIG_GRKERNSEC_RAND_THREADSTACK is not set
CONFIG_GRKERNSEC_PROC_MEMMAP=y
# CONFIG_GRKERNSEC_KSTACKOVERFLOW is not set
CONFIG_GRKERNSEC_BRUTE=y
CONFIG_GRKERNSEC_MODHARDEN=y
CONFIG_GRKERNSEC_HIDESYM=y
CONFIG_GRKERNSEC_RANDSTRUCT=y
CONFIG_GRKERNSEC_RANDSTRUCT_PERFORMANCE=y
# CONFIG_GRKERNSEC_NO_RBAC is not set
CONFIG_GRKERNSEC_ACL_HIDEKERN=y
CONFIG_GRKERNSEC_ACL_MAXTRIES=3
CONFIG_GRKERNSEC_ACL_TIMEOUT=30
# CONFIG_GRKERNSEC_PROC is not set
CONFIG_GRKERNSEC_LINK=y
# CONFIG_GRKERNSEC_SYMLINKOWN is not set
CONFIG_GRKERNSEC_FIFO=y
# CONFIG_GRKERNSEC_SYSFS_RESTRICT is not set
# CONFIG_GRKERNSEC_ROFS is not set
# CONFIG_GRKERNSEC_DEVICE_SIDECHANNEL is not set
# CONFIG_GRKERNSEC_CHROOT is not set
# CONFIG_GRKERNSEC_AUDIT_GROUP is not set
CONFIG_GRKERNSEC_EXECLOG=y
CONFIG_GRKERNSEC_RESLOG=y
CONFIG_GRKERNSEC_CHROOT_EXECLOG=y
CONFIG_GRKERNSEC_AUDIT_PTRACE=y
CONFIG_GRKERNSEC_AUDIT_CHDIR=y
CONFIG_GRKERNSEC_AUDIT_MOUNT=y
CONFIG_GRKERNSEC_SIGNAL=y
CONFIG_GRKERNSEC_FORKFAIL=y
CONFIG_GRKERNSEC_TIME=y
CONFIG_GRKERNSEC_PROC_IPADDR=y
CONFIG_GRKERNSEC_DMESG=y
CONFIG_GRKERNSEC_HARDEN_PTRACE=y
CONFIG_GRKERNSEC_PTRACE_READEXEC=y
CONFIG_GRKERNSEC_SETXID=y
CONFIG_GRKERNSEC_HARDEN_IPC=y
# CONFIG_GRKERNSEC_TPE is not set
CONFIG_GRKERNSEC_RANDNET=y
CONFIG_GRKERNSEC_BLACKHOLE=y
CONFIG_GRKERNSEC_NO_SIMULT_CONNECT=y
# CONFIG_GRKERNSEC_SOCKET is not set
CONFIG_GRKERNSEC_SYSCTL=y
# CONFIG_GRKERNSEC_SYSCTL_ON is not set
CONFIG_GRKERNSEC_FLOODTIME=10
CONFIG_GRKERNSEC_FLOODBURST=6
CONFIG_PAX_USERCOPY_SLABS=y
CONFIG_PAX=y
CONFIG_PAX_SOFTMODE=y
# CONFIG_PAX_EI_PAX is not set
# CONFIG_PAX_PT_PAX_FLAGS is not set
# CONFIG_PAX_XATTR_PAX_FLAGS is not set
CONFIG_PAX_NO_ACL_FLAGS=y
# CONFIG_PAX_HAVE_ACL_FLAGS is not set
# CONFIG_PAX_HOOK_ACL_FLAGS is not set
CONFIG_PAX_NOEXEC=y
CONFIG_PAX_PAGEEXEC=y
# CONFIG_PAX_MPROTECT is not set
CONFIG_PAX_KERNEXEC_PLUGIN_METHOD=""
CONFIG_PAX_ASLR=y
CONFIG_PAX_RANDUSTACK=y
CONFIG_PAX_RANDMMAP=y
CONFIG_PAX_MEMORY_SANITIZE=y
CONFIG_PAX_MEMORY_STRUCTLEAK=y
CONFIG_PAX_REFCOUNT=y
CONFIG_PAX_LATENT_ENTROPY=y

Re: I2C dev interface

PostPosted: Wed Jul 30, 2014 4:50 pm
by PaX Team
thanks, but i already found that code, i'm wondering specifically about the particular arguments passed to the failing memdup_user code, i.e., the values of data_ptrs[i] and rdwr_pa[i].len.

Re: I2C dev interface

PostPosted: Wed Jul 30, 2014 11:12 pm
by gregkmoff
Okay I got this from a couple of different runs:

Code: Select all
i2cdev_ioctl_rdrw:275 [0] data_ptrs[i]=0000000000660000 rdwr_pa[i].len=2
i2cdev_ioctl_rdrw:275 [0] data_ptrs[i]=00000000000b0000 rdwr_pa[i].len=2
i2cdev_ioctl_rdrw:275 [0] data_ptrs[i]=00000000003f0000 rdwr_pa[i].len=2

The values of data_ptrs doesn't seem right, but the length is. Could there be something wrong with this code:

Code: Select all
rdwr_pa = memdup_user(rdwr_arg.msgs, rdwr_arg.nmsgs * sizeof(struct i2c_msg));     

Re: I2C dev interface

PostPosted: Wed Jul 30, 2014 11:43 pm
by spender
What vanilla kernel were you using previously that worked successfully?

-Brad

Re: I2C dev interface

PostPosted: Thu Jul 31, 2014 12:02 am
by gregkmoff
It's worked before on both 3.10.28 and 3.4.34 vanilla kernels.

Re: I2C dev interface

PostPosted: Thu Jul 31, 2014 5:14 am
by PaX Team
what are the defintions of struct i2c_rdwr_ioctl_data and struct i2c_msg that your *userland* side of the code uses? also what should have been the correct userland pointer values?

Re: I2C dev interface

PostPosted: Thu Jul 31, 2014 7:48 am
by PaX Team
so i had a hunch that this might be something i fixed very recently, so can you give the latest patch a try please? also are you using mips by any chance (where this bug was reported to manifest)? here's the hunk for reference:
Code: Select all
--- a/fs/compat_ioctl.c  2014-06-09 12:46:23.625124694 +0200
+++ b/fs/compat_ioctl.c  2014-07-25 14:52:06.897166533 +0200
@@ -703,7 +703,7 @@
        for (i = 0; i < nmsgs; i++) {
                if (copy_in_user(&tmsgs[i].addr, &umsgs[i].addr, 3*sizeof(u16)))
                        return -EFAULT;
-               if (get_user(datap, (u8 __user * __user *)&umsgs[i].buf) ||
+               if (get_user(datap, (compat_caddr_t __user *)&umsgs[i].buf) ||
                    put_user(compat_ptr(datap), (u8 __user * __user *)&tmsgs[i].buf))
                        return -EFAULT;
        }

Re: I2C dev interface

PostPosted: Thu Jul 31, 2014 9:00 am
by gregkmoff
I checked the userspace side - it's definitions of the types are exactly the same as the kernel.

I am working with MIPS - good guess :-)

I tried the patch but that made no difference.

I did another test where we're sending a batch of messages at once passed to the ioctl. I dumped the contents of the msgs array and got this:

Code: Select all
[  167.450950] i2cdev_ioctl_rdrw:257 [0] addr=0x003f flags=0x0000 len=2 buf=00000000003f0000
[  167.549183] i2cdev_ioctl_rdrw:257 [1] addr=0x003f flags=0x0000 len=1 buf=00000000003f0001
[  167.647439] i2cdev_ioctl_rdrw:257 [2] addr=0x003f flags=0x0001 len=2 buf=00000000003f0000
[  167.746288] i2cdev_ioctl_rdrw:257 [3] addr=0x003f flags=0x0000 len=1 buf=00000000003f0001
[  167.844686] i2cdev_ioctl_rdrw:257 [4] addr=0x003f flags=0x0001 len=1 buf=00000000701ce7f0


I noticed two things:
a) only the last entry has a correct buf address. This is consistent with all my testing
b) if you look at the buf address for an entry, it seems to be the related to the next entry. For example, in my data about entry [1] has a buf of 3f0001. Entry [2] has an addr of 003f and a flags of 0001. In the struct i2c_msg definition both addr and flags are __u16. Is there something here that may indicate what's going on?

Re: I2C dev interface

PostPosted: Thu Jul 31, 2014 9:30 am
by gregkmoff
More data. Here's what I'm sending to the ioctl from userspace:

Code: Select all
[0] addr=0x003f flags=0x0000 len=2 buf=0x0x7d081720
[1] addr=0x003f flags=0x0000 len=1 buf=0x0x7d081722
[2] addr=0x003f flags=0x0001 len=2 buf=0x0x7d081723
[3] addr=0x003f flags=0x0000 len=1 buf=0x0x7d081725
[4] addr=0x003f flags=0x0001 len=1 buf=0x0x7d081726


and here's what we see in the kernel

Code: Select all
[  111.922339] i2cdev_ioctl_rdrw:257 [0] addr=0x003f flags=0x0000 len=2 buf=00000000003f0000
[  112.087004] i2cdev_ioctl_rdrw:257 [1] addr=0x003f flags=0x0000 len=1 buf=00000000003f0001
[  112.251370] i2cdev_ioctl_rdrw:257 [2] addr=0x003f flags=0x0001 len=2 buf=00000000003f0000
[  112.416211] i2cdev_ioctl_rdrw:257 [3] addr=0x003f flags=0x0000 len=1 buf=00000000003f0001
[  112.581304] i2cdev_ioctl_rdrw:257 [4] addr=0x003f flags=0x0001 len=1 buf=000000007484d7f0


Same behaviour as before...

Re: I2C dev interface

PostPosted: Thu Jul 31, 2014 9:47 am
by PaX Team
could you post the result of make fs/compat_ioctl.s?

Re: I2C dev interface

PostPosted: Thu Jul 31, 2014 12:49 pm
by gregkmoff
Code: Select all
  CC      fs/compat_ioctl.o
.../fs/compat_ioctl.c: In function 'do_i2c_rdwr_ioctl':
.../fs/compat_ioctl.c:706:7: warning: assignment makes integer from pointer without a cast [enabled by default]
.../fs/compat_ioctl.c:706:7: warning: assignment makes integer from pointer without a cast [enabled by default]
.../fs/compat_ioctl.c:706:7: warning: assignment makes integer from pointer without a cast [enabled by default]
.../fs/compat_ioctl.c:706:7: warning: assignment makes integer from pointer without a cast [enabled by default]


That's all I can find from the make logs.

Re: I2C dev interface

PostPosted: Thu Jul 31, 2014 1:25 pm
by PaX Team
i didn't mean fs/compat_ioctl.o but fs/compat_ioctl.s and i'll need the actual file (fs/compat_ioctl.s), not the make logs.

Re: I2C dev interface

PostPosted: Thu Jul 31, 2014 5:16 pm
by gregkmoff
Hmmm I have fs/compat_ioctl.c not fs/compat_ioctl.s

Is there another name for it?