From: "Zach, Yoav" This patch allows for misc binaries to run with credentials and security token that are calculated according to the binaries, and not according to the interpreter, which is the legacy behavior of binfmt_misc. The way it is done is by calling prepare_binprm, which is where these attributes are calculated, before switching the 'file' field in the bprm from the binary to the interpreter. This feature should be used with care, since the interpreter will have root permissions when running a setuid binary owned by root. Please note - - Only root can register an interpreter with binfmt_misc. The feature is documented and the administrator is advised to handle it with care - The new feature is enabled only with a special flag in the registration string. When this flag is not specified the current behavior of binfmt_misc is kept - This is the only 'right' way for an interpreter to know the correct AT_SECURE value for the interpreted binary From: Chris Wright This patchset looks OK, except for one problem. It installs the fd (which could've been unreadable) without unsharing the ->files. So someone can use this to read unreadable yet executable files. Here's a patch which fixes that up. I added one bit that's commented out because I'm not positive if a final steal_locks() is needed. I did a fair amount of rearranging to simplify the error conditions relative to the fd_install(), and unshare_files(). From: Chris Wright I found that the intel patchset (and mine as well) leaked i_writecount on the original executed file. In addition, I verified that the steal_locks() bit is indeed needed. Here is a fix relative to my last patch. --- 25-akpm/Documentation/binfmt_misc.txt | 7 + 25-akpm/fs/binfmt_misc.c | 122 +++++++++++++++++++--------------- 2 files changed, 78 insertions(+), 51 deletions(-) diff -puN Documentation/binfmt_misc.txt~binfmt_misc-credentials Documentation/binfmt_misc.txt --- 25/Documentation/binfmt_misc.txt~binfmt_misc-credentials 2004-05-09 21:40:14.277152232 -0700 +++ 25-akpm/Documentation/binfmt_misc.txt 2004-05-09 21:40:14.282151472 -0700 @@ -48,6 +48,13 @@ Here is what the fields mean: the interpreter to execute non-readable binaries. This feature should be used with care - the interpreter has to be trusted not to emit the contents of the non-readable binary. + 'C' - credentials. Currently, the behavior of binfmt_misc is to calculate + the credentials and security token of the new process according to + the interpreter. When this flag is included, these attributes are + calculated according to the binary. It also implies the 'O' flag. + This feature should be used with care as the interpreter + will run with root permissions when a setuid binary owned by root + is run with binfmt_misc. There are some restrictions: diff -puN fs/binfmt_misc.c~binfmt_misc-credentials fs/binfmt_misc.c --- 25/fs/binfmt_misc.c~binfmt_misc-credentials 2004-05-09 21:40:14.278152080 -0700 +++ 25-akpm/fs/binfmt_misc.c 2004-05-09 21:40:24.843545896 -0700 @@ -40,6 +40,7 @@ static int enabled = 1; enum {Enabled, Magic}; #define MISC_FMT_PRESERVE_ARGV0 (1<<31) #define MISC_FMT_OPEN_BINARY (1<<30) +#define MISC_FMT_CREDENTIALS (1<<29) typedef struct { struct list_head list; @@ -104,14 +105,12 @@ static int load_misc_binary(struct linux { Node *fmt; struct file * interp_file = NULL; - struct file * binary_file = NULL; char iname[BINPRM_BUF_SIZE]; char *iname_addr = iname; int retval; int fd_binary = -1; - char fd_str[32]; - char * fdsp = fd_str; - int is_open_bin; + char fd_str[12]; + struct files_struct *files = NULL; retval = -ENOEXEC; if (!enabled) @@ -126,39 +125,56 @@ static int load_misc_binary(struct linux if (!fmt) goto _ret; - is_open_bin = (fmt->flags & MISC_FMT_OPEN_BINARY) ? 1 : 0; + if (!(fmt->flags & MISC_FMT_PRESERVE_ARGV0)) { + remove_arg_zero(bprm); + } - if (is_open_bin) { + if (fmt->flags & MISC_FMT_OPEN_BINARY) { + char *fdsp = fd_str; + + files = current->files; + retval = unshare_files(); + if (retval < 0) + goto _ret; + if (files == current->files) { + put_files_struct(files); + files = NULL; + } /* if the binary should be opened on behalf of the * interpreter than keep it open and assign descriptor * to it */ - fd_binary = get_unused_fd (); + fd_binary = get_unused_fd(); if (fd_binary < 0) { retval = fd_binary; - goto _ret; + goto _unshare; } - snprintf (fd_str, sizeof(fd_str) - 1, "%d", fd_binary); - } else { - allow_write_access (bprm->file); - fput (bprm->file); - bprm->file = NULL; - } + fd_install(fd_binary, bprm->file); - /* Build args for interpreter */ - if (!(fmt->flags & MISC_FMT_PRESERVE_ARGV0)) { - remove_arg_zero(bprm); - } + /* if the binary is not readable than enforce mm->dumpable=0 + regardless of the interpreter's permissions */ + if (permission(bprm->file->f_dentry->d_inode, MAY_READ, NULL)) + bprm->interp_flags |= BINPRM_FLAGS_ENFORCE_NONDUMP; + + allow_write_access(bprm->file); + bprm->file = NULL; - if (is_open_bin) { /* make argv[1] be the file descriptor of the binary */ - retval = copy_strings_kernel (1, &fdsp, bprm); + snprintf(fd_str, sizeof(fd_str), "%d", fd_binary); + retval = copy_strings_kernel(1, &fdsp, bprm); + if (retval < 0) + goto _error; + bprm->argc++; + } else { + allow_write_access(bprm->file); + fput(bprm->file); + bprm->file = NULL; /* make argv[1] be the path to the binary */ retval = copy_strings_kernel (1, &bprm->interp, bprm); + if (retval < 0) + goto _error; + bprm->argc++; } - if (retval < 0) - goto _error; - bprm->argc ++; retval = copy_strings_kernel (1, &iname_addr, bprm); if (retval < 0) goto _error; @@ -170,47 +186,41 @@ static int load_misc_binary(struct linux if (IS_ERR (interp_file)) goto _error; - - binary_file = bprm->file; bprm->file = interp_file; - retval = prepare_binprm(bprm); + if (fmt->flags & MISC_FMT_CREDENTIALS) { + /* + * No need to call prepare_binprm(), it's already been + * done. bprm->buf is stale, update from interp_file. + */ + memset(bprm->buf, 0, BINPRM_BUF_SIZE); + retval = kernel_read(bprm->file, 0, bprm->buf, BINPRM_BUF_SIZE); + } else + retval = prepare_binprm (bprm); if (retval < 0) goto _error; - if (is_open_bin) { - /* if the binary is not readable than enforce mm->dumpable=0 - regardless of the interpreter's permissions */ - if (permission (binary_file->f_dentry->d_inode, MAY_READ, NULL)) { - bprm->interp_flags |= BINPRM_FLAGS_ENFORCE_NONDUMP; - } - /* install the binary's fd. it is done at the latest possible point - * because once it is installed it will need to be sys_close()ed - * in case of error. - */ - fd_install (fd_binary, binary_file); - } - retval = search_binary_handler (bprm, regs); - if (retval < 0) - goto _error_close_file; + goto _error; + if (files) { + steal_locks(files); + put_files_struct(files); + files = NULL; + } _ret: return retval; - -_error_close_file: - if (fd_binary > 0) { - sys_close (fd_binary); - fd_binary = -1; - bprm->file = NULL; - } _error: if (fd_binary > 0) - put_unused_fd (fd_binary); + sys_close(fd_binary); bprm->interp_flags = 0; +_unshare: + if (files) { + put_files_struct(current->files); + current->files = files; + } goto _ret; - } /* Command parsers */ @@ -271,6 +281,13 @@ static inline char * check_special_flags p++; e->flags |= MISC_FMT_OPEN_BINARY; break; + case 'C': + p++; + /* this flags also implies the + open-binary flag */ + e->flags |= (MISC_FMT_CREDENTIALS | + MISC_FMT_OPEN_BINARY); + break; default: cont = 0; } @@ -280,7 +297,7 @@ static inline char * check_special_flags } /* * This registers a new binary format, it recognises the syntax - * ':name:type:offset:magic:mask:interpreter:' + * ':name:type:offset:magic:mask:interpreter:flags' * where the ':' is the IFS, that can be chosen with the first char */ static Node *create_entry(const char *buffer, size_t count) @@ -453,6 +470,9 @@ static void entry_status(Node *e, char * if (e->flags & MISC_FMT_OPEN_BINARY) { *dp ++ = 'O'; } + if (e->flags & MISC_FMT_CREDENTIALS) { + *dp ++ = 'C'; + } *dp ++ = '\n'; _