On Wed, Jun 15, 2011 at 04:04:14PM +0200, Thorsten Kukuk wrote:
On Wed, Jun 15, Kees Cook wrote:
> On Wed, Jun 15, 2011 at 03:33:54PM +0200, Thorsten Kukuk wrote:
> > On Thu, Mar 31, Kees Cook wrote:
> >
> > > Since the kernel sets a number of dynamic rlimits based on the system
> > > properties (e.g. physical memory for nproc), these rlimits should be
> > > respected by PAM. Parse /proc/1/limits for the kernel-defined rlimits.
> >
> > Ok, attached is a patch, where I fixed the syntax errors and I
> > added an "init_all" option. Means by default no behavior changes
> > against the current version. If admin specifies "init_all", the
missing
> > limits will be set from the one of process with PId 1.
>
> Thanks, this looks good. I made one additional change since sending this
> originally to reduce the syslog spam on some distros that are behind on
> their rlimits headers:
>
> > + i = str2rlimit(name);
> > + if (i < 0 || i >= RLIM_NLIMITS) {
>
> if (ctrl & PAM_DEBUG_ARG)
Ok, added.
> > + pam_syslog(pamh, LOG_DEBUG, "Unknown kernel rlimit
'%s' ignored", name);
> > + continue;
> > + }
>
>
> > +#ifdef __linux__
> > + if (ctrl & PAM_INIT_ALL) {
>
> Should this be called PAM_INIT_ALL_ARG instead?
Why should it? No other option has the _ARG suffix besice
the PAM_DEBUG_ARG, and that has only to avoid a symbol conflict
with libpam headers itself.
Okay, cool. I didn't go check the others. Thanks!
-Kees
--
Kees Cook
Ubuntu Security Team