LWN.net Logo

Advertisement

GStreamer, Embedded Linux, Android, VoD, Smooth Streaming, DRM, RTSP, HEVC, PulseAudio, OpenGL. Register now to attend.

Advertise here

Re: [PATCH v2] vfs: Tighten up linkat(..., AT_EMPTY_PATH)

From:  Linus Torvalds <torvalds-AT-linux-foundation.org>
To:  Andy Lutomirski <luto-AT-amacapital.net>
Subject:  Re: [PATCH v2] vfs: Tighten up linkat(..., AT_EMPTY_PATH)
Date:  Thu, 22 Aug 2013 11:48:10 -0700
Message-ID:  <CA+55aFzXNU-eJ7KeZyv5YYKKtm8iorG-o3QtD7NXx0GTzuH+Bw@mail.gmail.com>
Cc:  "security-AT-kernel.org" <security-AT-kernel.org>, Ingo Molnar <mingo-AT-kernel.org>, Willy Tarreau <w-AT-1wt.eu>, Linux Kernel Mailing List <linux-kernel-AT-vger.kernel.org>, Oleg Nesterov <oleg-AT-redhat.com>, Al Viro <viro-AT-zeniv.linux.org.uk>, Linux FS Devel <linux-fsdevel-AT-vger.kernel.org>, Brad Spengler <spender-AT-grsecurity.net>
Archive-link:  Article, Thread

On Wed, Aug 21, 2013 at 12:14 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>
> So let's be careful for now: only allow linkat(..., AT_EMPTY_PATH)
> if the target is I_LINKABLE.

So I really detest this just because it's such a special case. Now
this is only useful for that one special case, and the thing very
fundamentally checks that one special case in a place that is
impossible to check for the /proc case, so the proc case remains
totally separate.

Which just bothers me.

I think we could easily at least allow the "file->f_creds ==
current->creds" case (and yes, I literally mean comparing the pointers
- not only is it cheaper, but it literally means "nothing odd has
happened in between opening and the lookup").

And I'm wondering if we shouldn't actually do that at "path_init"
time. Right now the code says:

                /* Caller must check execute permissions on the
starting path component */
                struct fd f = fdget_raw(dfd);

and then uses the struct file mindlessly.

I'm wondering if we should just do some validation in that place, and say:

 - for directories, we require exec permissions here
 - for everything else, we require that f->f_cred == current->cred check.

I dunno. But that I_LINKABLE thing just bothers me. It screams "I'm
hacky" to me.

               Linus


(Log in to post comments)

Copyright © 2013, Eklektix, Inc.
Comments and public postings are copyrighted by their creators.
Linux is a registered trademark of Linus Torvalds