diff mbox series

[06/11] vfs: copy_file_range needs to strip setuid bits

Message ID 20181203083416.28978-7-david@fromorbit.com (mailing list archive)
State New, archived
Headers show
Series fs: fixes for major copy_file_range() issues | expand

Commit Message

Dave Chinner Dec. 3, 2018, 8:34 a.m. UTC 
From: Dave Chinner <dchinner@redhat.com>

The file we are copying data into needs to have its setuid bit
stripped before we start the data copy so that unprivileged users
can't copy data into executables that are run with root privs.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 fs/read_write.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

Comments

Amir Goldstein Dec. 3, 2018, 12:51 p.m. UTC | #1 
On Mon, Dec 3, 2018 at 10:34 AM Dave Chinner <david@fromorbit.com> wrote:
>
> From: Dave Chinner <dchinner@redhat.com>
>
> The file we are copying data into needs to have its setuid bit
> stripped before we start the data copy so that unprivileged users
> can't copy data into executables that are run with root privs.
>
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---

Looks good.
Reviewed-by: Amir Goldstein <amir73il@gmail.com>

Thanks,
Amir.
Christoph Hellwig Dec. 4, 2018, 3:21 p.m. UTC | #2 
file_remove_privs needs to be called with i_rsem held, which I don't
think we do here.  It also really should be called under the same
i_rwsem critical section that does modify the file content, so we'll
have to move the call into the methods.
diff mbox series

Patch

diff --git a/fs/read_write.c b/fs/read_write.c
index 69809345977e..3b101183ea19 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -1574,6 +1574,16 @@  static ssize_t do_copy_file_range(struct file *file_in, loff_t pos_in,
 			    struct file *file_out, loff_t pos_out,
 			    size_t len, unsigned int flags)
 {
+	ssize_t ret;
+
+	/*
+	 * Clear the security bits if the process is not being run by root.
+	 * This keeps people from modifying setuid and setgid binaries.
+	 */
+	ret = file_remove_privs(file_out);
+	if (ret)
+		return ret;
+
 	if (file_out->f_op->copy_file_range)
 		return file_out->f_op->copy_file_range(file_in, pos_in, file_out,
 						      pos_out, len, flags);