|
 Home
|
|
|
PR Database Search
|
From dholland@netbsd.org Thu Nov 26 02:58:15 2015
Return-Path: <dholland@netbsd.org>
Received: from mail.netbsd.org (mail.netbsd.org [149.20.53.66])
(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
(Client CN "mail.netbsd.org", Issuer "Postmaster NetBSD.org" (verified OK))
by mollari.NetBSD.org (Postfix) with ESMTPS id 44059A5864
for <gnats-bugs@gnats.NetBSD.org>; Thu, 26 Nov 2015 02:58:15 +0000 (UTC)
Message-Id: <20151126025815.0310F14A282@mail.netbsd.org>
Date: Thu, 26 Nov 2015 02:58:15 +0000 (UTC)
From: dholland@netbsd.org
Reply-To: dholland@netbsd.org
To: gnats-bugs@gnats.NetBSD.org
Subject: sys_issetugid is missing locking
X-Send-Pr-Version: 3.95
>Number: 50475
>Category: kern
>Synopsis: sys_issetugid is missing locking
>Confidential: no
>Severity: non-critical
>Priority: medium
>Responsible: kern-bug-people
>State: open
>Class: sw-bug
>Submitter-Id: net
>Arrival-Date: Thu Nov 26 03:00:00 +0000 2015
>Last-Modified: Sat Nov 28 23:15:00 +0000 2015
>Originator: David A. Holland
>Release: NetBSD 7.99.21 (20151118)
>Organization:
>Environment:
System: NetBSD valkyrie 7.99.1 NetBSD 7.99.1 (VALKYRIE) #17: Wed Oct 14 03:21:03 EDT 2015 dholland@valkyrie:/usr/src/sys/arch/amd64/compile/VALKYRIE amd64
Architecture: x86_64
Machine: amd64
>Description:
sys_issetugid() reads curproc->p_flag without taking any locks, but
sys/proc.h says p_flag is supposed to be protected by p_lock.
Granted reading ints is atomic on normal platforms, but it's wrong,
and also other code is entitled to assume that it can temporarily
leave invalid values in p_flag while it holds p_lock.
If the behavior of sys_issetugid() is intended, it should be
documented in proc.h.
>How-To-Repeat:
code inspection
>Fix:
Take the mutex in sys_issetugid(); the cost of doing so in a call
that's not on anything's critical path isn't worth stressing about.
Alternatively, update the locking documentation in sys/proc.h.
Preferably also find other similar cases at the same time.
Hopefully, don't spend a week arguing about locking overhead.
>Audit-Trail:
From: matthew green <mrg@eterna.com.au>
To: gnats-bugs@NetBSD.org
Cc: kern-bug-people@netbsd.org, gnats-admin@netbsd.org,
netbsd-bugs@netbsd.org
Subject: re: kern/50475: sys_issetugid is missing locking
Date: Sun, 29 Nov 2015 10:13:54 +1100
> sys_issetugid() reads curproc->p_flag without taking any locks, but
> sys/proc.h says p_flag is supposed to be protected by p_lock.
>
> Granted reading ints is atomic on normal platforms, but it's wrong,
> and also other code is entitled to assume that it can temporarily
> leave invalid values in p_flag while it holds p_lock.
>
> If the behavior of sys_issetugid() is intended, it should be
> documented in proc.h.
i think this is entirely intended. these values are readable
in process context without a lock. it's not just PK_SUGID.
this happens all over the tree, and i think it's entirely
reasonable.
proc.h comments need updating only here, i think.
.mrg.
|
|