| 1 |
Hi and thanks for your answer, in rsbac code in namei.c this code: |
| 2 |
|
| 3 |
rsbac_name = rsbac_symlink_redirect(dentry- |
| 4 |
>d_inode, link, buflen); |
| 5 |
|
| 6 |
assigns to rsbac_name the result of rsbac_symlink_redirect() |
| 7 |
|
| 8 |
the part I have found about rsbac_symlink_redirect definition is this (from |
| 9 |
adf_main.c, rsbac only code) |
| 10 |
|
| 11 |
( |
| 12 |
http://git.rsbac.org/cgi-bin/gitweb.cgi?p=linux-3.8.y.git;a=blob;f=rsbac/adf/adf_main.c;h=decb72b3648cf4353deead1b880048bbfa17a035;hb=HEAD: |
| 13 |
|
| 14 |
#ifdef CONFIG_RSBAC_SYM_REDIR |
| 15 |
2741 EXPORT_SYMBOL(rsbac_symlink_redirect); |
| 16 |
2742 |
| 17 |
2743 /* This function changes the symlink content by adding a suffix, if |
| 18 |
2744 * requested. It returns NULL, if unchanged, or a pointer to a |
| 19 |
2745 * kmalloc'd new char * otherwise, which has to be kfree'd after use. |
| 20 |
2746 */ |
| 21 |
2747 *char * rsbac_symlink_redirect( |
| 22 |
2748 struct inode * inode_p, |
| 23 |
2749 const char * name, |
| 24 |
2750 u_int maxlen)* |
| 25 |
2751 { |
| 26 |
2752 #if defined(CONFIG_RSBAC_SYM_REDIR_REMOTE_IP) || |
| 27 |
defined(CONFIG_RSBAC_SYM_REDIR_MAC) || defined(CONFIG_RSBAC_SYM_REDIR_RC) |
| 28 |
|| defined(CONFIG_RSBAC_SYM_REDIR_UID) |
| 29 |
2753 * union rsbac_target_id_t * i_tid_p; |
| 30 |
2754 int err; |
| 31 |
2755 union rsbac_attribute_value_t i_attr_val;* |
| 32 |
2756 #endif |
| 33 |
. |
| 34 |
. |
| 35 |
. |
| 36 |
#if defined(CONFIG_RSBAC_SYM_REDIR_REMOTE_IP) || |
| 37 |
defined(CONFIG_RSBAC_SYM_REDIR_MAC) || defined(CONFIG_RSBAC_SYM_REDIR_RC) |
| 38 |
|| defined(CONFIG_RSBAC_SYM_REDIR_UID) |
| 39 |
2793 * i_tid_p = kmalloc(sizeof(*i_tid_p), GFP_KERNEL);* |
| 40 |
2794 if(!i_tid_p) |
| 41 |
2795 { |
| 42 |
2796 rsbac_printk(KERN_DEBUG |
| 43 |
2797 "rsbac_symlink_redirect(): not enough memory for symlink |
| 44 |
redir remote ip inode %u on dev %02u:%02u!\n", |
| 45 |
2798 inode_p->i_ino, |
| 46 |
2799 RSBAC_MAJOR(inode_p->i_sb->s_dev), |
| 47 |
RSBAC_MINOR(inode_p->i_sb->s_dev) ); |
| 48 |
2800 return NULL; |
| 49 |
2801 } |
| 50 |
2802 i_tid_p->symlink.device = inode_p->i_sb->s_dev; |
| 51 |
2803 i_tid_p->symlink.inode = inode_p->i_ino; |
| 52 |
2804 i_tid_p->symlink.dentry_p = NULL; |
| 53 |
2805 #endif |
| 54 |
|
| 55 |
|
| 56 |
So, Would be safe maintain the namei.c related part from fixation patch as |
| 57 |
is isn't it? |
| 58 |
|
| 59 |
This in particular: |
| 60 |
|
| 61 |
#ifdef CONFIG_RSBAC_SYM_REDIR |
| 62 |
rsbac_name = rsbac_symlink_redirect(dentry->d_inode, link, buflen); |
| 63 |
if (rsbac_name) { |
| 64 |
len = strlen(rsbac_name); |
| 65 |
if (copy_to_user(buffer, rsbac_name, len)) |
| 66 |
len = -EFAULT; |
| 67 |
kfree(rsbac_name); |
| 68 |
} |
| 69 |
else |
| 70 |
#endif |
| 71 |
if (len < sizeof(tmpbuf)) { |
| 72 |
memcpy(tmpbuf, link, len); |
| 73 |
newlink = tmpbuf; |
| 74 |
} else |
| 75 |
newlink = link; |
| 76 |
|
| 77 |
if (copy_to_user(buffer, newlink, len)) |
| 78 |
len = -EFAULT; |
| 79 |
out: |
| 80 |
return len; |
| 81 |
} |
| 82 |
|
| 83 |
This piece of code doesn't change usually change in rsbac as I would had |
| 84 |
seen, so fixation patch should stay equal towards (if switched correct PaX |
| 85 |
patch and rsbac patch it only rejects in this four positions and always the |
| 86 |
same ones, so fixation patch should work for another versions too.. |
| 87 |
|
| 88 |
Thanks a lot pageexec. |
| 89 |
|
| 90 |
|
| 91 |
2013/7/29 PaX Team <pageexec@×××××.com> |
| 92 |
|
| 93 |
> On 29 Jul 2013 at 6:23, Javier Juan Martínez Cabezón wrote: |
| 94 |
> |
| 95 |
> > PaX tries to do this modification to rsbac git code: |
| 96 |
> > |
| 97 |
> > --- fs/namei.c 2013-03-19 01:53:21.091281869 +0100 |
| 98 |
> > +++ fs/namei.c 2013-03-19 01:53:31.251281326 +0100 |
| 99 |
> > @@ -3954,7 +3956,14 @@ |
| 100 |
> > len = strlen(link); |
| 101 |
> > if (len > (unsigned) buflen) |
| 102 |
> > len = buflen; |
| 103 |
> > - if (copy_to_user(buffer, link, len)) |
| 104 |
> > + |
| 105 |
> > + if (len < sizeof(tmpbuf)) { |
| 106 |
> > + memcpy(tmpbuf, link, len); |
| 107 |
> > + newlink = tmpbuf; |
| 108 |
> > + } else |
| 109 |
> > + newlink = link; |
| 110 |
> > + |
| 111 |
> > + if (copy_to_user(buffer, newlink, len)) |
| 112 |
> > len = -EFAULT; |
| 113 |
> > out: |
| 114 |
> > return len; |
| 115 |
> |
| 116 |
> this change is done for USERCOPY to prevent false positive reports when the |
| 117 |
> name comes from a dentry field (vs. a normal kmalloc slab) or something |
| 118 |
> like that. if you want to enable USERCOPY under RSBAC as well then you'll |
| 119 |
> have to ensure that either rsbac_name is allocated by a normal kmalloc |
| 120 |
> (this |
| 121 |
> seems to be the case already from a quick look) or you'll have to do the |
| 122 |
> temporary stack copy as done in the above snippet. |
| 123 |
> |
| 124 |
> |
| 125 |
> |
| 126 |
> |
Archives