136 lines
4.7 KiB
Diff
136 lines
4.7 KiB
Diff
|
commit 584bf002ec62a333840b87193b93ee5a521063f7
|
||
|
Author: J. R. Okajima <hooanon05@yahoo.co.jp>
|
||
|
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 <hooanon05@yahoo.co.jp>
|
||
|
|
||
|
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);
|
||
|
}
|
||
|
}
|
||
|
|