All of lore.kernel.org
 help / color / mirror / Atom feed
From: Willy Tarreau <w@1wt.eu>
To: Kees Cook <keescook@chromium.org>
Cc: linux-kernel@vger.kernel.org,
	Andrew Morton <akpm@linux-foundation.org>,
	Dave Chinner <david@fromorbit.com>,
	Andy Lutomirski <luto@amacapital.net>, Jan Kara <jack@suse.cz>,
	"Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
	Mel Gorman <mgorman@suse.de>,
	Johannes Weiner <hannes@cmpxchg.org>,
	Rik van Riel <riel@redhat.com>,
	Matthew Wilcox <willy@linux.intel.com>,
	Shachar Raindel <raindel@mellanox.com>,
	Boaz Harrosh <boaz@plexistor.com>, Michal Hocko <mhocko@suse.cz>,
	Haggai Eran <haggaie@mellanox.com>,
	Theodore Tso <tytso@google.com>,
	Dirk Steinmetz <public@rsjtdrjgfuzkfg.com>,
	Michael Kerrisk-manpages <mtk.manpages@gmail.com>,
	Serge Hallyn <serge.hallyn@ubuntu.com>,
	Seth Forshee <seth.forshee@canonical.com>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	Linux FS Devel <linux-fsdevel@vger.kernel.org>,
	"Eric W . Biederman" <ebiederm@xmission.com>,
	Serge Hallyn <serge.hallyn@canonical.com>,
	linux-mm@kvack.org
Subject: Re: [PATCH] fs: clear file set[ug]id when writing via mmap
Date: Fri, 20 Nov 2015 02:00:16 +0100	[thread overview]
Message-ID: <20151120010016.GB31694@1wt.eu> (raw)
In-Reply-To: <20151120001043.GA28204@www.outflux.net>

Hi Kees,

On Thu, Nov 19, 2015 at 04:10:43PM -0800, Kees Cook wrote:
> Normally, when a user can modify a file that has setuid or setgid bits,
> those bits are cleared when they are not the file owner or a member of the
> group. This is enforced when using write() directly but not when writing
> to a shared mmap on the file. This could allow the file writer to gain
> privileges by changing the binary without losing the setuid/setgid bits.
> 
> Signed-off-by: Kees Cook <keescook@chromium.org>
> Cc: stable@vger.kernel.org
> ---
>  mm/memory.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/mm/memory.c b/mm/memory.c
> index deb679c31f2a..4c970a4e0057 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -2036,6 +2036,7 @@ static inline int wp_page_reuse(struct mm_struct *mm,
>  
>  		if (!page_mkwrite)
>  			file_update_time(vma->vm_file);
> +		file_remove_privs(vma->vm_file);

I thought you said in one of the early mails of this thread that it
didn't work. Or maybe I misunderstood.

Also, don't you think we should move that into the if (!page_mkwrite)
just like for the time update ?

Willy


WARNING: multiple messages have this Message-ID (diff)
From: Willy Tarreau <w@1wt.eu>
To: Kees Cook <keescook@chromium.org>
Cc: linux-kernel@vger.kernel.org,
	Andrew Morton <akpm@linux-foundation.org>,
	Dave Chinner <david@fromorbit.com>,
	Andy Lutomirski <luto@amacapital.net>, Jan Kara <jack@suse.cz>,
	"Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
	Mel Gorman <mgorman@suse.de>,
	Johannes Weiner <hannes@cmpxchg.org>,
	Rik van Riel <riel@redhat.com>,
	Matthew Wilcox <willy@linux.intel.com>,
	Shachar Raindel <raindel@mellanox.com>,
	Boaz Harrosh <boaz@plexistor.com>, Michal Hocko <mhocko@suse.cz>,
	Haggai Eran <haggaie@mellanox.com>,
	Theodore Tso <tytso@google.com>,
	Dirk Steinmetz <public@rsjtdrjgfuzkfg.com>,
	Michael Kerrisk-manpages <mtk.manpages@gmail.com>,
	Serge Hallyn <serge.hallyn@ubuntu.com>,
	Seth Forshee <seth.forshee@canonical.com>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	Linux FS Devel <linux-fsdevel@vger.kernel.org>,
	"Eric W . Biederman" <ebiederm@xmission.com>,
	Serge Hallyn <serge.hallyn@canonical.com>,
	linux-mm@kvack.org
Subject: Re: [PATCH] fs: clear file set[ug]id when writing via mmap
Date: Fri, 20 Nov 2015 02:00:16 +0100	[thread overview]
Message-ID: <20151120010016.GB31694@1wt.eu> (raw)
In-Reply-To: <20151120001043.GA28204@www.outflux.net>

Hi Kees,

On Thu, Nov 19, 2015 at 04:10:43PM -0800, Kees Cook wrote:
> Normally, when a user can modify a file that has setuid or setgid bits,
> those bits are cleared when they are not the file owner or a member of the
> group. This is enforced when using write() directly but not when writing
> to a shared mmap on the file. This could allow the file writer to gain
> privileges by changing the binary without losing the setuid/setgid bits.
> 
> Signed-off-by: Kees Cook <keescook@chromium.org>
> Cc: stable@vger.kernel.org
> ---
>  mm/memory.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/mm/memory.c b/mm/memory.c
> index deb679c31f2a..4c970a4e0057 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -2036,6 +2036,7 @@ static inline int wp_page_reuse(struct mm_struct *mm,
>  
>  		if (!page_mkwrite)
>  			file_update_time(vma->vm_file);
> +		file_remove_privs(vma->vm_file);

I thought you said in one of the early mails of this thread that it
didn't work. Or maybe I misunderstood.

Also, don't you think we should move that into the if (!page_mkwrite)
just like for the time update ?

Willy

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  parent reply	other threads:[~2015-11-20  1:00 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-20  0:10 [PATCH] fs: clear file set[ug]id when writing via mmap Kees Cook
2015-11-20  0:10 ` Kees Cook
2015-11-20  0:41 ` Andrew Morton
2015-11-20  0:41   ` Andrew Morton
2015-11-20  0:52   ` Kees Cook
2015-11-20  0:52     ` Kees Cook
2015-11-20  1:00 ` Willy Tarreau [this message]
2015-11-20  1:00   ` Willy Tarreau
2015-11-20  1:03   ` Willy Tarreau
2015-11-20  1:03     ` Willy Tarreau
2015-11-20  1:03   ` Kees Cook
2015-11-20  1:03     ` Kees Cook
2015-11-20  1:06     ` Willy Tarreau
2015-11-20  1:06       ` Willy Tarreau
2015-11-23 12:26 ` Jan Kara
2015-11-23 12:26   ` Jan Kara
2015-11-23 12:34   ` Eric W. Biederman
2015-11-23 12:34     ` Eric W. Biederman
2015-11-23 12:34     ` Eric W. Biederman
2015-12-02 23:55   ` Kees Cook
2015-12-02 23:55     ` Kees Cook

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20151120010016.GB31694@1wt.eu \
    --to=w@1wt.eu \
    --cc=akpm@linux-foundation.org \
    --cc=boaz@plexistor.com \
    --cc=david@fromorbit.com \
    --cc=ebiederm@xmission.com \
    --cc=haggaie@mellanox.com \
    --cc=hannes@cmpxchg.org \
    --cc=jack@suse.cz \
    --cc=keescook@chromium.org \
    --cc=kirill.shutemov@linux.intel.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=luto@amacapital.net \
    --cc=mgorman@suse.de \
    --cc=mhocko@suse.cz \
    --cc=mtk.manpages@gmail.com \
    --cc=public@rsjtdrjgfuzkfg.com \
    --cc=raindel@mellanox.com \
    --cc=riel@redhat.com \
    --cc=serge.hallyn@canonical.com \
    --cc=serge.hallyn@ubuntu.com \
    --cc=seth.forshee@canonical.com \
    --cc=tytso@google.com \
    --cc=viro@zeniv.linux.org.uk \
    --cc=willy@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.