--- fs/binfmt_script.c 2012-01-19 23:04:48.000000000 +0000 +++ fs/binfmt_script.c 2012-08-19 07:08:42.540611605 +0000 @@ -14,12 +14,24 @@ #include #include +/** Check if this handler is suitable to load the "binary" identified + * by first BINPRM_BUF_SIZE bytes in bprm->buf. + * @returns -ENOEXEC if this handler is not suitable for that type + * of binary. In that case, the handler must not modify any of the + * data associated with bprm. + * Any error if the binary should have been handled by this loader + * but handling failed. In that case. FIXME: be defensive? also + * kill bprm->mm or bprm->file also to make it impossible, that + * upper search_binary_handler can continue handling? + * 0 (OK) otherwise, the new executable is ready in bprm->mm. + */ static int load_script(struct linux_binprm *bprm,struct pt_regs *regs) { const char *i_arg, *i_name; char *cp; struct file *file; - char interp[BINPRM_BUF_SIZE]; + char bprm_buf_copy[BINPRM_BUF_SIZE]; + char *bprm_old_interp_name; int retval; if ((bprm->buf[0] != '#') || (bprm->buf[1] != '!') || @@ -30,25 +42,29 @@ static int load_script(struct linux_binp * Sorta complicated, but hopefully it will work. -TYT */ - bprm->recursion_depth++; - allow_write_access(bprm->file); - fput(bprm->file); - bprm->file = NULL; + /* Keep bprm unchanged until we known, that this is a script + * to be handled by this loader. Copy bprm->buf for sure, + * otherwise returning -ENOEXEC will make other handlers see + * modified data. (hd) + */ + memcpy(bprm_buf_copy, bprm->buf, BINPRM_BUF_SIZE); - bprm->buf[BINPRM_BUF_SIZE - 1] = '\0'; - if ((cp = strchr(bprm->buf, '\n')) == NULL) - cp = bprm->buf+BINPRM_BUF_SIZE-1; + bprm_buf_copy[BINPRM_BUF_SIZE - 1]='\0'; + if ((cp = strchr(bprm_buf_copy, '\n')) == NULL) + cp = bprm_buf_copy+BINPRM_BUF_SIZE-1; *cp = '\0'; - while (cp > bprm->buf) { + while (cp > bprm_buf_copy) { cp--; if ((*cp == ' ') || (*cp == '\t')) *cp = '\0'; else break; } - for (cp = bprm->buf+2; (*cp == ' ') || (*cp == '\t'); cp++); + for (cp = bprm_buf_copy+2; (*cp == ' ') || (*cp == '\t'); cp++); if (*cp == '\0') - return -ENOEXEC; /* No interpreter name found */ + /* No interpreter name found. No problem to let other handlers + * retry, we did not change anything. */ + return -ENOEXEC; i_name = cp; i_arg = NULL; for ( ; *cp && (*cp != ' ') && (*cp != '\t'); cp++) @@ -57,45 +73,83 @@ static int load_script(struct linux_binp *cp++ = '\0'; if (*cp) i_arg = cp; - strcpy (interp, i_name); + + /* So this is our point-of-no-return: modification of bprm + * will be irreversible, so if we fail to setup execution + * using the new interpreter name (i_name), we have to make + * sure, that no other handler tries again. (hd) + */ + /* * OK, we've parsed out the interpreter name and * (optional) argument. * Splice in (1) the interpreter's name for argv[0] - * (2) (optional) argument to interpreter - * (3) filename of shell script (replace argv[0]) + * (2) (optional) argument to interpreter + * (3) filename of shell script (replace argv[0]) * * This is done in reverse order, because of how the * user environment and arguments are stored. */ + + /* Ugly: we store pointer to local stack frame in bprm, + * so make sure to clear this up before returning. + */ + bprm_old_interp_name = bprm->interp; + bprm->interp = i_name; + retval = remove_arg_zero(bprm); - if (retval) - return retval; - retval = copy_strings_kernel(1, &bprm->interp, bprm); - if (retval < 0) return retval; + if (retval) goto out; + /* copy_strings_kernel is ok here, even when racy: since no + * user can be attached to new mm, there is nobody to race + * with and call is safe for now. And copy_strings_kernel + * cannot return -ENOEXEC in any case. (hd) + */ + retval = copy_strings_kernel(1, &bprm_old_interp_name, bprm); + if (retval < 0) goto out; bprm->argc++; if (i_arg) { retval = copy_strings_kernel(1, &i_arg, bprm); - if (retval < 0) return retval; + if (retval < 0) goto out; bprm->argc++; } - retval = copy_strings_kernel(1, &i_name, bprm); - if (retval) return retval; + retval = copy_strings_kernel(1, &bprm->interp, bprm); + if (retval) goto out; bprm->argc++; - bprm->interp = interp; /* * OK, now restart the process with the interpreter's dentry. + * Release old file first */ - file = open_exec(interp); - if (IS_ERR(file)) - return PTR_ERR(file); - + allow_write_access(bprm->file); + fput(bprm->file); + bprm->file = NULL; + file = open_exec(bprm->interp); + if (IS_ERR(file)) { + retval=PTR_ERR(file); + goto out; + } bprm->file = file; + /* Caveat: This also updates the credentials of the next exec. */ retval = prepare_binprm(bprm); if (retval < 0) - return retval; - return search_binary_handler(bprm,regs); + goto out; + bprm->recursion_depth++; + retval=search_binary_handler(bprm,regs); + +out: /* Make sure, we do not return local stack frame data. If + * it would be needed after returning, we would have needed + * to allocate memory or use copy from new bprm->mm anyway. (hd) + */ + bprm->interp = bprm_old_interp_name; + if(!retval) { + /* The handlers for starting of interpreter failed. + * bprm is already modified, hence we are dead here. + * Make sure, that we do not return -ENOEXEC, that would + * allow searching for handlers to continue. (hd). + */ + if(retval==-ENOEXEC) retval=-EINVAL; + } + return(retval); } static struct linux_binfmt script_format = {