oss-sec mailing list archives
Re: [PATCH 1/3] setup_arg_pages: diagnose excessive argument size
From: Roland McGrath <roland () redhat com>
Date: Fri, 10 Sep 2010 02:18:30 -0700 (PDT)
I still don't think this addresses the whole problem.
You're responding to one of three patches, and I said to begin with that all together they only address part of the problem (not the OOM part). So the import of this remark is somewhat mysterious.
Without question, the rlimit / 4 check is bogus.
I question that assertion. For a non-RLIM_INFINITY limit, there is nothing in particular wrong with it. The kernel is free to pick its upper bound for ARG_MAX by whatever method. I don't see anything much to object to about the rlimit/4 method. It has no useful effect for RLIM_INFINITY and IMHO should not try to impose any limit in that case. But that's the only thing I see a reason to change.
If nobody agrees with the intent of that check, then it should be removed, but I think the better solution is to fix the check so that it matches its original intent: let the initial stack setup be up to 1/Xth of the min(rlimit, TASK_SIZE dependent upon personality), which allows space for additional stack setup in the ELF loader and then further growth once the process is live.
I see no reason to suspect this was the "original intent". It seems most likely to me that the original intent was 1/4th the RLIMIT_STACK size, and just nobody thought about what that meant when RLIMIT_STACK was RLIM_INFINITY.
If that amount is overstepped, then the exec will return an error to the calling process instead of being terminated.
That's what happens now when RLIMIT_STACK is smaller, and that's what people really care about. What you suggest would require some more significant changes to the exec code path, touching all the binfmt modules (though probably only binfmt_elf matters). In the current structure of the code, the arch-dependent SET_PERSONALITY macro used by {compat_,}binfmt_elf is the only place that knows what arch bits to set for the new address space size. This is itself destructive, but also runs after flush_old_exec (the point of no return). So you'd have to reorganize things significantly, or add an entirely new arch macro tied into struct binfmt somehow, or something like that.
It might be useful to consult with the people who introduced/approved the check in the first place, as they seemed to have reasons for implementing it.
This was done in commit b6a2fea by Ollie Wild <aaw () google com>: mm: variable length argument support It was part of going from a fixed maximum to no fixed maximum. The log includes: [a.p.zijlstra () chello nl: limit stack size] So perhaps it was Peter who devised the rlimit/4 idea. Thanks, Roland
Current thread:
- Re: [PATCH 1/3] setup_arg_pages: diagnose excessive argument size, (continued)
- Re: [PATCH 1/3] setup_arg_pages: diagnose excessive argument size Roland McGrath (Sep 14)
- Re: [PATCH 1/3] setup_arg_pages: diagnose excessive argument size pageexec (Sep 14)
- Re: [PATCH 1/3] setup_arg_pages: diagnose excessive argument size Roland McGrath (Sep 14)
- Re: [PATCH 1/3] setup_arg_pages: diagnose excessive argument size pageexec (Sep 14)
- Re: [PATCH 1/3] setup_arg_pages: diagnose excessive argument size Roland McGrath (Sep 14)
- Re: [PATCH 1/3] setup_arg_pages: diagnose excessive argument size pageexec (Sep 15)
- Message not available
- Re: [PATCH 1/3] setup_arg_pages: diagnose excessive argument size Roland McGrath (Sep 10)
- Re: [PATCH 1/3] setup_arg_pages: diagnose excessive argument size pageexec (Sep 11)
- Re: [PATCH 1/3] setup_arg_pages: diagnose excessive argument size Roland McGrath (Sep 14)
- Re: [PATCH 1/3] setup_arg_pages: diagnose excessive argument size pageexec (Sep 14)
- Message not available
- Re: [PATCH 1/3] setup_arg_pages: diagnose excessive argument size Roland McGrath (Sep 10)
- [PATCH 2/3] execve: improve interactivity with large arguments Roland McGrath (Sep 07)
- [PATCH 3/3] execve: make responsive to SIGKILL with large arguments Roland McGrath (Sep 07)
- Re: [PATCH 0/3] execve argument-copying fixes KOSAKI Motohiro (Sep 07)
- [PATCH 0/2] execve memory exhaust of argument-copying fixes KOSAKI Motohiro (Sep 09)
- [PATCH 1/2] oom: don't ignore rss in nascent mm KOSAKI Motohiro (Sep 09)
- Message not available
- Re: [PATCH 1/2] oom: don't ignore rss in nascent mm Roland McGrath (Sep 10)
- Message not available
- [PATCH] move cred_guard_mutex from task_struct to signal_struct KOSAKI Motohiro (Sep 10)
- Re: [PATCH] move cred_guard_mutex from task_struct to signal_struct Oleg Nesterov (Sep 10)
- Re: [PATCH] move cred_guard_mutex from task_struct to signal_struct KOSAKI Motohiro (Sep 15)
- [PATCH 2/2] execve: check the VM has enough memory at first KOSAKI Motohiro (Sep 09)