Date: Mon, 9 Jan 2012 11:58:20 -0800 (PST) From: Don Lewis <truckman@FreeBSD.org> To: des@des.no Cc: current@FreeBSD.org Subject: Re: couldn't log on to my -CURRENT machine after upgrade to latest PAM Message-ID: <201201091958.q09JwKZK029815@gw.catspoiler.org> In-Reply-To: <86ty44ykj4.fsf@ds4.des.no>
next in thread | previous in thread | raw e-mail | index | archive | help
On 9 Jan, Dag-Erling Smørgrav wrote: > Don Lewis <truckman@FreeBSD.org> writes: >> Dag-Erling Smørgrav <des@des.no> writes: >> > The culprit was this commit: >> > >> > http://trac.des.no/openpam/changeset/487/trunk/lib/openpam_configure.c >> > >> > However, I'm not confident that simply reverting this commit is the >> > right way to go. >> Thanks for the detective work. It looks to me like the bug is caused by >> the change in the openpam_parse_chain() return value. In the previous >> code it returned the value of count, which I would guess was greater >> than zero if it found something. In that case, the for loop in >> openpam_load_chain() would be terminated because r != 0. In the new >> code, openpam_parse_chain() will return PAM_SUCCESS if it found >> something, and the loop in openpam_load_chain() will go through another >> iteration because ret == PAM_SUCCESS. > > Thank you, Captain Obvious. I am still not confident that simply > reverting this commit is the right way to go, because it discards > valuable information when an error occurs, especially if an error occurs > while parsing an include. It wasn't so obvious to me, especially with the gratuitous variable renaming in the diff. After staring at the code a lot more, I see your point about the loss of information. The problem is that openpam_parse_chain() returns PAM_SUCCESS whether or not if found anything, but we want the loop to terminate when either an error is detected or if openpam_parse_chain() actually found something. Maybe changing the loop exit to something like this would work: if (ret != PAM_SUCCESS || pamh->chains[facility] != NULL) return (ret);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201201091958.q09JwKZK029815>