From owner-svn-src-all@FreeBSD.ORG Sat Mar 29 12:31:36 2014 Return-Path: Delivered-To: svn-src-all@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id E65B0B3E; Sat, 29 Mar 2014 12:31:36 +0000 (UTC) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:1900:2254:206c::16:87]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id B7D28BBD; Sat, 29 Mar 2014 12:31:36 +0000 (UTC) Received: from [127.0.0.1] (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.8/8.14.8) with ESMTP id s2TCVWPd034887; Sat, 29 Mar 2014 12:31:33 GMT (envelope-from davidxu@freebsd.org) Message-ID: <5336BD22.1040906@freebsd.org> Date: Sat, 29 Mar 2014 20:31:30 +0800 From: David Xu User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:24.0) Gecko/20100101 Thunderbird/24.2.0 MIME-Version: 1.0 To: Don Lewis , mjguzik@gmail.com Subject: Re: svn commit: r263755 - head/sys/kern References: <201403290752.s2T7qldY012467@gw.catspoiler.org> In-Reply-To: <201403290752.s2T7qldY012467@gw.catspoiler.org> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: kostikbel@gmail.com, svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org, mjg@FreeBSD.org X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 29 Mar 2014 12:31:37 -0000 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. >>>> >>> 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. >>> >> 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. >> >> 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. > > > 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. > > 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.