Re: [gentoo-hardened] rsbac+pax fixation Patch to kernel 3.8

[Javier Juan Martínez Cabezón]

Tomwij, blueness, as rsbac_sources maintainers, if you want to test
fixation Patch with the source I have pointed in my initial mail, expect
some troubles in compilation (conflicting types with k_uidt. I think it's
not related with fixation Patch, but with rsbac instead, and git in
particular an something puntual :-S.  So when I could make it compile (I
have asked about this to ao, tested with rsbac 3.10 too...with same result)
I will tell you,

Rsbac git is now (as I have seen this night) with troubles and I could only
get rsbac sources from webgit clicking in "snapshot"





2013/7/29 Javier Juan Martínez Cabezón <tazok.id0@×××××.com>

> Hi and thanks for your answer, in rsbac code in namei.c  this code:
>
>
>  rsbac_name = rsbac_symlink_redirect(dentry-
> >d_inode, link, buflen);
>
> assigns to rsbac_name the result of rsbac_symlink_redirect()
>
> the part I have found about rsbac_symlink_redirect definition is this
> (from adf_main.c, rsbac only code)
>
> (
> 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:
>
> #ifdef CONFIG_RSBAC_SYM_REDIR
> 2741 EXPORT_SYMBOL(rsbac_symlink_redirect);
> 2742
> 2743 /* This function changes the symlink content by adding a suffix, if
> 2744  * requested. It returns NULL, if unchanged, or a pointer to a
> 2745  * kmalloc'd new char * otherwise, which has to be kfree'd after use.
> 2746  */
> 2747 *char * rsbac_symlink_redirect(
> 2748   struct inode * inode_p,
> 2749   const char * name,
> 2750   u_int maxlen)*
> 2751   {
> 2752 #if defined(CONFIG_RSBAC_SYM_REDIR_REMOTE_IP) ||
> defined(CONFIG_RSBAC_SYM_REDIR_MAC) || defined(CONFIG_RSBAC_SYM_REDIR_RC)
> || defined(CONFIG_RSBAC_SYM_REDIR_UID)
> 2753   *  union rsbac_target_id_t * i_tid_p;
> 2754     int err;
> 2755     union rsbac_attribute_value_t i_attr_val;*
> 2756 #endif
> .
> .
> .
>  #if defined(CONFIG_RSBAC_SYM_REDIR_REMOTE_IP) ||
> defined(CONFIG_RSBAC_SYM_REDIR_MAC) || defined(CONFIG_RSBAC_SYM_REDIR_RC)
> || defined(CONFIG_RSBAC_SYM_REDIR_UID)
> 2793   *  i_tid_p = kmalloc(sizeof(*i_tid_p), GFP_KERNEL);*
> 2794     if(!i_tid_p)
> 2795       {
> 2796         rsbac_printk(KERN_DEBUG
> 2797            "rsbac_symlink_redirect(): not enough memory for symlink
> redir remote ip inode %u on dev %02u:%02u!\n",
> 2798            inode_p->i_ino,
> 2799            RSBAC_MAJOR(inode_p->i_sb->s_dev),
> RSBAC_MINOR(inode_p->i_sb->s_dev) );
> 2800         return NULL;
> 2801       }
> 2802     i_tid_p->symlink.device = inode_p->i_sb->s_dev;
> 2803     i_tid_p->symlink.inode = inode_p->i_ino;
> 2804     i_tid_p->symlink.dentry_p = NULL;
> 2805 #endif
>
>
> So,  Would be safe maintain the namei.c related part from fixation patch
> as is isn't it?
>
> This in particular:
>
>
> #ifdef CONFIG_RSBAC_SYM_REDIR
>     rsbac_name = rsbac_symlink_redirect(dentry->d_inode, link, buflen);
>     if (rsbac_name) {
>         len = strlen(rsbac_name);
>         if (copy_to_user(buffer, rsbac_name, len))
>             len = -EFAULT;
>         kfree(rsbac_name);
>     }
>     else
> #endif
>     if (len < sizeof(tmpbuf)) {
>         memcpy(tmpbuf, link, len);
>         newlink = tmpbuf;
>     } else
>         newlink = link;
>
>     if (copy_to_user(buffer, newlink, len))
>         len = -EFAULT;
> out:
>     return len;
> }
>
> This piece of code doesn't change usually change in rsbac as I would had
> seen, so fixation patch should stay equal towards (if switched correct PaX
> patch and rsbac patch it only rejects in this four positions and always the
> same ones, so fixation patch should work for another versions too..
>
> Thanks a lot pageexec.
>
>
>
> 2013/7/29 PaX Team <pageexec@×××××.com>
>
>> On 29 Jul 2013 at 6:23, Javier Juan Martínez Cabezón wrote:
>>
>> > PaX tries to do this modification to rsbac git code:
>> >
>> > --- fs/namei.c    2013-03-19 01:53:21.091281869 +0100
>> > +++ fs/namei.c    2013-03-19 01:53:31.251281326 +0100
>> > @@ -3954,7 +3956,14 @@
>> >      len = strlen(link);
>> >      if (len > (unsigned) buflen)
>> >          len = buflen;
>> > -    if (copy_to_user(buffer, link, len))
>> > +
>> > +    if (len < sizeof(tmpbuf)) {
>> > +        memcpy(tmpbuf, link, len);
>> > +        newlink = tmpbuf;
>> > +    } else
>> > +        newlink = link;
>> > +
>> > +    if (copy_to_user(buffer, newlink, len))
>> >          len = -EFAULT;
>> >  out:
>> >      return len;
>>
>> this change is done for USERCOPY to prevent false positive reports when
>> the
>> name comes from a dentry field (vs. a normal kmalloc slab) or something
>> like that. if you want to enable USERCOPY under RSBAC as well then you'll
>> have to ensure that either rsbac_name is allocated by a normal kmalloc
>> (this
>> seems to be the case already from a quick look) or you'll have to do the
>> temporary stack copy as done in the above snippet.
>>
>>
>>
>>
>