* [PATCH][REPOST] Check maxlen on strnlen_user usage @ 2011-09-07 23:54 Ryan Mallon 2011-09-08 0:06 ` Andrew Morton 0 siblings, 1 reply; 3+ messages in thread From: Ryan Mallon @ 2011-09-07 23:54 UTC (permalink / raw) To: Mark Salter, Alexander Viro Cc: linux-fsdevel, linux-kernel@vger.kernel.org, Andrew Morton, trivial@kernel.org strnlen_user returns the length of the string including the nul terminator. In the case where maxlen is reached strnlen_user returns maxlen + 1. Most callsites already check for this condition. Fix the call to strnlen_user in fs/exec.c to check for the maxlen case. Signed-off-by: Ryan Mallon <rmallon@gmail.com> --- diff --git a/fs/exec.c b/fs/exec.c index 25dcbe5..e19588c 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -481,7 +481,7 @@ static int copy_strings(int argc, struct user_arg_ptr argv, goto out; len = strnlen_user(str, MAX_ARG_STRLEN); - if (!len) + if (!len || len> MAX_ARG_STRLEN) goto out; ret = -E2BIG; ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH][REPOST] Check maxlen on strnlen_user usage 2011-09-07 23:54 [PATCH][REPOST] Check maxlen on strnlen_user usage Ryan Mallon @ 2011-09-08 0:06 ` Andrew Morton 2011-09-08 0:20 ` Ryan Mallon 0 siblings, 1 reply; 3+ messages in thread From: Andrew Morton @ 2011-09-08 0:06 UTC (permalink / raw) To: Ryan Mallon Cc: Mark Salter, Alexander Viro, linux-fsdevel, linux-kernel@vger.kernel.org, trivial@kernel.org On Thu, 08 Sep 2011 09:54:18 +1000 Ryan Mallon <rmallon@gmail.com> wrote: > strnlen_user returns the length of the string including the nul > terminator. In the case where maxlen is reached strnlen_user returns > maxlen + 1. Most callsites already check for this condition. Fix the > call to strnlen_user in fs/exec.c to check for the maxlen case. > > diff --git a/fs/exec.c b/fs/exec.c > index 25dcbe5..e19588c 100644 > --- a/fs/exec.c > +++ b/fs/exec.c > @@ -481,7 +481,7 @@ static int copy_strings(int argc, struct > user_arg_ptr argv, > goto out; > > len = strnlen_user(str, MAX_ARG_STRLEN); > - if (!len) > + if (!len || len> MAX_ARG_STRLEN) > goto out; > > ret = -E2BIG; The following call to vald_arg_len() already does this? This change will cause copy_strings() to incorrectly return -EFAULT, rather than -E2BIG. Your email client is wordwrapping and space-stuffing the patches, btw. ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH][REPOST] Check maxlen on strnlen_user usage 2011-09-08 0:06 ` Andrew Morton @ 2011-09-08 0:20 ` Ryan Mallon 0 siblings, 0 replies; 3+ messages in thread From: Ryan Mallon @ 2011-09-08 0:20 UTC (permalink / raw) To: Andrew Morton Cc: Mark Salter, Alexander Viro, linux-fsdevel, linux-kernel@vger.kernel.org, trivial@kernel.org On 08/09/11 10:06, Andrew Morton wrote: > On Thu, 08 Sep 2011 09:54:18 +1000 > Ryan Mallon<rmallon@gmail.com> wrote: > >> strnlen_user returns the length of the string including the nul >> terminator. In the case where maxlen is reached strnlen_user returns >> maxlen + 1. Most callsites already check for this condition. Fix the >> call to strnlen_user in fs/exec.c to check for the maxlen case. >> >> diff --git a/fs/exec.c b/fs/exec.c >> index 25dcbe5..e19588c 100644 >> --- a/fs/exec.c >> +++ b/fs/exec.c >> @@ -481,7 +481,7 @@ static int copy_strings(int argc, struct >> user_arg_ptr argv, >> goto out; >> >> len = strnlen_user(str, MAX_ARG_STRLEN); >> - if (!len) >> + if (!len || len> MAX_ARG_STRLEN) >> goto out; >> >> ret = -E2BIG; > The following call to vald_arg_len() already does this? > > This change will cause copy_strings() to incorrectly return -EFAULT, > rather than -E2BIG. Hmm, missed that. How about the patch below instead which passes the correct length to strnlen_user? > Your email client is wordwrapping and space-stuffing the patches, btw. I'll try again. Hopefully it works this time. --- Replace valid_arg_len function in fs/exec.c with max_arg_len function and pass the correct length to strnlen_user. Signed-off-by: Ryan Mallon <rmallon@gmail.com> --- diff --git a/fs/exec.c b/fs/exec.c index e19588c..12d2f7f 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -296,9 +296,9 @@ err: return err; } -static bool valid_arg_len(struct linux_binprm *bprm, long len) +static long max_arg_len(struct linux_binprm *bprm) { - return len<= MAX_ARG_STRLEN; + return MAX_ARG_STRLEN; } #else @@ -354,9 +354,9 @@ static int __bprm_mm_init(struct linux_binprm *bprm) return 0; } -static bool valid_arg_len(struct linux_binprm *bprm, long len) +static long max_arg_len(struct linux_binprm *bprm) { - return len<= bprm->p; + return bprm->p; } #endif /* CONFIG_MMU */ @@ -474,18 +474,19 @@ static int copy_strings(int argc, struct user_arg_ptr argv, const char __user *str; int len; unsigned long pos; + long max_len = max_arg_len(bprm); ret = -EFAULT; str = get_user_arg_ptr(argv, argc); if (IS_ERR(str)) goto out; - len = strnlen_user(str, MAX_ARG_STRLEN); - if (!len || len> MAX_ARG_STRLEN) + len = strnlen_user(str, max_len); + if (!len) goto out; ret = -E2BIG; - if (!valid_arg_len(bprm, len)) + if (len> max_len) goto out; /* We're going to work our way backwords. */ ^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2011-09-08 0:20 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-09-07 23:54 [PATCH][REPOST] Check maxlen on strnlen_user usage Ryan Mallon 2011-09-08 0:06 ` Andrew Morton 2011-09-08 0:20 ` Ryan Mallon
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).