Date: Sat, 16 Jan 2016 23:43:13 +0100 From: Mateusz Guzik <mjguzik@gmail.com> To: Vijay Singh <vijju.singh@gmail.com> Cc: Konstantin Belousov <kostikbel@gmail.com>, freebsd-hackers@freebsd.org, Chagin Dmitry <dchagin@freebsd.org> Subject: Re: irrelevant locking Message-ID: <20160116224312.GA1963@dft-labs.eu> In-Reply-To: <CALCNsJT_gH5gJaB%2ByVQRcON84JntSUevG8-X-0Z5_13DkPC%2BBg@mail.gmail.com> References: <20160116195819.GA41610@chd.heemeyer.club> <20160116202643.GL3942@kib.kiev.ua> <CALCNsJT_gH5gJaB%2ByVQRcON84JntSUevG8-X-0Z5_13DkPC%2BBg@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sat, Jan 16, 2016 at 02:08:58PM -0800, Vijay Singh wrote:
> Couldn't the get & set race otherwise?
Current locking plays no role in correctness here.
Say that locking is left in place. A concurrent setgroups (or whatever)
call resulting in setting P_SUGID is also being executed. Regardless of
whether PROC_LOCK/PROC_UNLOCK pair is in place, it can set the bit
before or after it is being tested by sys_issetugid.
In principle, the very moment you drop a lock, your informatoin is
stale.
This does not matter here. It's only the process itself which can set
the bit, so it would have to race with itself.
Finally, the bit can be only unset during execve, which cannot be
executed here - if it is being executed, there is only one thread doing
work and, well, it is doing execve.
The real question is if it would make sense to add the bit to elf aux
vector to save the call as done by the loader.
> On Jan 16, 2016 12:27 PM, "Konstantin Belousov" <kostikbel@gmail.com> wrote:
>
> > On Sat, Jan 16, 2016 at 10:58:19PM +0300, Chagin Dmitry wrote:
> > > hi, please, can someone explain the reason to take the process lock here:
> > There is no reason, I think that the PROC_LOCK() can be removed.
> >
> > >
> > > int
> > > sys_issetugid(register struct thread *td, struct issetugid_args *uap)
> > > {
> > > struct proc *p = td->td_proc;
> > >
> > > /*
> > > * Note: OpenBSD sets a P_SUGIDEXEC flag set at execve() time,
> > > * we use P_SUGID because we consider changing the owners as
> > > * "tainting" as well.
> > > * This is significant for procs that start as root and "become"
> > > * a user without an exec - programs cannot know *everything*
> > > * that libc *might* have put in their data segment.
> > > */
> > > PROC_LOCK(p);
> > > td->td_retval[0] = (p->p_flag & P_SUGID) ? 1 : 0;
> > > PROC_UNLOCK(p);
> > > return (0);
> > > }
> > _______________________________________________
> > freebsd-hackers@freebsd.org mailing list
> > https://lists.freebsd.org/mailman/listinfo/freebsd-hackers
> > To unsubscribe, send any mail to "freebsd-hackers-unsubscribe@freebsd.org"
> >
> _______________________________________________
> freebsd-hackers@freebsd.org mailing list
> https://lists.freebsd.org/mailman/listinfo/freebsd-hackers
> To unsubscribe, send any mail to "freebsd-hackers-unsubscribe@freebsd.org"
--
Mateusz Guzik <mjguzik gmail.com>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20160116224312.GA1963>