[lkml]   [2012]   [Jul]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: [PATCH 1/2] fs: add link restrictions
    On Sat, Jun 30, 2012 at 2:14 AM, Al Viro <> wrote:
    > On Mon, Jun 25, 2012 at 02:05:26PM -0700, Kees Cook wrote:
    >> +config PROTECTED_LINKS
    >> + bool "Evaluate vulnerable link conditions"
    >> + default y
    > Remember Linus' rants about 'default y' in general?

    I added these configs due to other people's requests. I am happy to
    remove them all and have the sysctls start their life == 1. It would
    eliminate all the #ifdef logic too.

    >> + /* Check parent directory mode and owner. */
    > I suspect that we ought to simply pass that parent directory as argument - caller
    > *does* have a reference to it, so we don't need to mess with ->d_lock, etc.

    I don't see where the parent is held in either path_openat nor
    path_lookupat. What should I be passing into may_follow_link() for the

    >> + err = may_follow_link(&link);
    >> + if (unlikely(err))
    >> + break;
    > No. This is definitely wrong - you are leaking dentries and vfsmount here.

    What should I do to avoid the leak? I thought it was avoiding the need
    to call put_link because it aborts before calling follow_link.

    >> + error = may_follow_link(&link);
    >> + if (unlikely(error))
    >> + break;
    > Ditto.

    Same thing here -- it aborts before the follow_link. I must be
    misunderstanding something. What am I missing?

    Thanks for the feedback! I'll clean up the other things you mentioned as well.


    Kees Cook
    Chrome OS Security

     \ /
      Last update: 2012-07-02 03:21    [W:0.026 / U:43.736 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site