commit 584bf002ec62a333840b87193b93ee5a521063f7 Author: J. R. Okajima Date: Thu May 27 11:28:41 2010 +0900 aufs: dynop supports grsec/pax patch The grsec/pax patches make member of struct brabra_operation 'const.' I don't understand why they need these 'const'. They modifies some of structures, but other structures. What do they want to protect from what? The keyword 'const' is essentially a feature of C language and it never modifes the behaviour of software. It just prohibits the assignment (or modification) to a variable which is expected not to be modified. In other word, it is a feature for programmers and doesn't enhance the security level. Actually programmers can bypass 'const' easily by indirect assignment as this patch does. Also the grsec/pax patches modifies some assignments to the member of struct brabra_operation in mainline kernel, but they don't make the confirmation fot that. For example, they replaced these assignments by declaring a structure statically. - /* inherit and extend fuse_dev_operations */ - cuse_channel_fops = fuse_dev_operations; - cuse_channel_fops.owner = THIS_MODULE; - cuse_channel_fops.open = cuse_channel_open; - cuse_channel_fops.release = cuse_channel_release; +static const struct file_operations cuse_channel_fops = { + .owner = THIS_MODULE, + .llseek = no_llseek, + .read = do_sync_read, + .aio_read = fuse_dev_read, + .write = do_sync_write, + .aio_write = fuse_dev_write, + .poll = fuse_dev_poll, + .open = cuse_channel_open, + .release = cuse_channel_release, + .fasync = fuse_dev_fasync, +}; By this modification, there exists major possible future problem I am afraid. _If_ fuse_dev_operations is modified, then this code needs to follow the change. But it is hard to detect such modification since there is no trick to do so. Generally it is recommended to put code such like this. ---------------------------------------------------------------------- int n; n++; BUG_ON(super.member != derive.member); } while (0); n++; /* owner */ MakeSure(fuse_dev_operations, cuse_channel_fops, llseek); MakeSure(fuse_dev_operations, cuse_channel_fops, read); ::: BUG_ON(n != sizeof(cuse_channel_fops)/sizeof(cuse_channel_fops.owner)); ---------------------------------------------------------------------- This piece of code ensures two things. - cuse_channel_fops correctly inherits fuse_dev_operations, eg. all members are equivalent except the overrided ones. - if some members are added or deleted from struct file_operations, it should be detected by a debugging feature, the variable 'n'. Without such trick, I am afraid the simple modification is a regression. Signed-off-by: J. R. Okajima diff --git a/fs/aufs/dynop.c b/fs/aufs/dynop.c index 12ea894..109d3bb 100644 --- a/fs/aufs/dynop.c +++ b/fs/aufs/dynop.c @@ -146,11 +146,22 @@ void au_dy_put(struct au_dykey *key) #define DyDbgInc(cnt) do {} while (0) #endif +#define AuGrsecPaxPtr(func, dst, src) do { \ + union { \ + const void *o; \ + char **p; \ + } u; \ + BUILD_BUG_ON(sizeof(u.o) != sizeof(&dst.func)); \ + BUILD_BUG_ON(sizeof(*u.p) != sizeof(src.func)); \ + u.o = (void *)&dst.func; \ + *u.p = (void *)src.func; \ +} while (0) + #define DySet(func, dst, src, h_op, h_sb) do { \ DyDbgInc(cnt); \ if (h_op->func) { \ if (src.func) \ - dst.func = src.func; \ + AuGrsecPaxPtr(func, dst, src); \ else \ AuDbg("%s %s\n", au_sbtype(h_sb), #func); \ } \ @@ -159,7 +170,7 @@ void au_dy_put(struct au_dykey *key) #define DySetForce(func, dst, src) do { \ AuDebugOn(!src.func); \ DyDbgInc(cnt); \ - dst.func = src.func; \ + AuGrsecPaxPtr(func, dst, src); \ } while (0) #define DySetAop(func) \ @@ -297,14 +308,21 @@ out: */ static void dy_adx(struct au_dyaop *dyaop, int do_dx) { + union { + void *direct_IO, *get_xip_mem; + } grsec_pax_dummy = { + .get_xip_mem = NULL + }; + if (!do_dx) { - dyaop->da_op.direct_IO = NULL; - dyaop->da_op.get_xip_mem = NULL; + AuGrsecPaxPtr(direct_IO, dyaop->da_op, grsec_pax_dummy); + AuGrsecPaxPtr(get_xip_mem, dyaop->da_op, grsec_pax_dummy); } else { - dyaop->da_op.direct_IO = aufs_aop.direct_IO; - dyaop->da_op.get_xip_mem = aufs_aop.get_xip_mem; + AuGrsecPaxPtr(direct_IO, dyaop->da_op, aufs_aop); + AuGrsecPaxPtr(get_xip_mem, dyaop->da_op, aufs_aop); if (!dyaop->da_get_xip_mem) - dyaop->da_op.get_xip_mem = NULL; + AuGrsecPaxPtr(get_xip_mem, dyaop->da_op, + grsec_pax_dummy); } }