From owner-svn-src-head@FreeBSD.ORG Sat Mar 29 15:42:16 2014 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id D69B4A24 for ; Sat, 29 Mar 2014 15:42:16 +0000 (UTC) Received: from mail-pd0-f174.google.com (mail-pd0-f174.google.com [209.85.192.174]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id A3543E3C for ; Sat, 29 Mar 2014 15:42:16 +0000 (UTC) Received: by mail-pd0-f174.google.com with SMTP id y13so6085201pdi.19 for ; Sat, 29 Mar 2014 08:42:10 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:sender:content-type:mime-version:subject:from :in-reply-to:date:cc:content-transfer-encoding:message-id:references :to; bh=adU1kMR6mtYwsfX//GKZmacJA3I1fI3XnjaxyKrUgI4=; b=KvRIridKmtHJH7O/fHBw2zrbQqkoDgtDNOSUtnZk915OejszbuRhtgxDwzM9yO6a7l Vd4nC5BHDed/7k2A89IM9v0xkcl5hmkljDNgK6GEGDttT29boqdjLpSfLnQunRFisFCy bY28LhoxRGytBM82Ng2XU1qWw85M5U5H/msk1tpiSrQD00AClb5gEOn5bf6sW6OQZn+S K5vTQiPNbff/3iBoZoIFF+76/32wvh423i0odFTjwFrlQ4T+9b+5FZrsHVRK/UVnEL7a 1Xv3Mq1u4K+nHUtSH/ERjKjbzmgON6LjBY4nxr/vIQ5imbJ67G6uwANw5dqYM9oIgTAI Sdsg== X-Gm-Message-State: ALoCoQkLT95Gin1lzj1Sd/ZWaTPWwwafA/mkHvD4Fh7XWHdjTUgQOCoJlJ8VUviihKKpjSQFMJyO X-Received: by 10.68.163.197 with SMTP id yk5mr14830483pbb.57.1396107730690; Sat, 29 Mar 2014 08:42:10 -0700 (PDT) Received: from [10.64.26.19] (dc1-prod.netflix.com. [69.53.236.251]) by mx.google.com with ESMTPSA id ac5sm32134407pbc.37.2014.03.29.08.42.08 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Sat, 29 Mar 2014 08:42:10 -0700 (PDT) Sender: Warner Losh Content-Type: text/plain; charset=windows-1252 Mime-Version: 1.0 (Mac OS X Mail 7.2 \(1874\)) Subject: Re: svn commit: r263755 - head/sys/kern From: Warner Losh In-Reply-To: <5336BD22.1040906@freebsd.org> Date: Sat, 29 Mar 2014 09:42:08 -0600 Content-Transfer-Encoding: quoted-printable Message-Id: References: <201403290752.s2T7qldY012467@gw.catspoiler.org> <5336BD22.1040906@freebsd.org> To: David Xu X-Mailer: Apple Mail (2.1874) Cc: Mateusz Guzik , Mateusz Guzik , Don Lewis , svn-src-head@FreeBSD.org, src-committers@FreeBSD.org, kostikbel@gmail.com, svn-src-all@FreeBSD.org X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 29 Mar 2014 15:42:17 -0000 On Mar 29, 2014, at 6:31 AM, David Xu wrote: >=20 > On 2014/03/29 15:52, Don Lewis wrote: >> On 29 Mar, Mateusz Guzik wrote: >>> On Sat, Mar 29, 2014 at 11:52:09AM +0800, David Xu wrote: >>>>> If fsetown handling like this is insecure this would bite us in = that >>>>> scenario (and few others). In short, if we can avoid giving = another way >>>>> to corrupt stuff in the kernel to userspace, we should. >>>>>=20 >>>> I can not see what you said, where is the security problem with = fsetown ? >>>> if you have per-jail instance of devsoftc, they all are operating = on their >>>> own instance. but I don't think this patch should address jail now, = there >>>> are many things are not jail ready. >>>>=20 >>> I asked if multpiple concurrent calls to fsetown(.., &pointer) could >>> result in some corruption in the kernel - if they could, we would = have a >>> problem in the future. >>>=20 >>> I decided to read the code once more and fsetown seems to be safe in >>> this regard after all and with that in mind the patch looks good to = me. >> =20 >> The fsetown() implementation does sufficient locking to prevent the >> kernel from getting into a bad state. The issue is that the device = can >> only have at most one owner (which may be a process group). If = multiple >> processes are allowed to open the device, or if a process that opened >> the device shares the descriptor with another process, the last call = to >> fsetown() wins. That means that one process could steal ownership = from >> another if they both have the same device open. >>=20 >> The reason that I suggested checking ownership when handling FIOASYNC = is >> that in the case of two processes sharing access to a device, there = is >> currently nothing that prevents a non-owner of the device from = enabling >> this mode and causing SIGIO signals to be sent to the owner, which = might >> not be expecting to receive them. > I think if you add ownership checking, it will be incompatible with > other code, people have to change their mind when dealing with > this special file descriptor, my recommendation is people don't need > to refresh their brain. > OTOH, if it is a problem, we should have already been flooded by > the problem, but in the past years, I saw zero complaining in the = mailing > lists. I believe that the SIGIO code was cut and pasted from a driver I was = working on in the 4.x time frame. devd is the only consumer, and it doesn=92t do = the FIOASYNC stuff at all. So I=92d be strongly biased to either (a) remove support for this or (b) = make the support correct, even at the cost of speed or performance. Warner=