Re: rsbac+pax fixation Patch to kernel 3.8

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 <[email protected]>

> 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.
>
>
>
>