From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jiri Slaby Subject: Re: [patch 2/4] mISDN: add support for group membership check Date: Fri, 20 Sep 2013 18:14:28 +0200 Message-ID: <523C7464.3070007@suse.cz> References: <20130913215202.7D16C31C1BF@corp2gmr1-1.hot.corp.google.com> <1379201336.19779.35.camel@deadeye.wl.decadent.org.uk> <523C5141.4080608@suse.cz> <20130920.115634.1931451843075283025.davem@davemloft.net> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: ben@decadent.org.uk, akpm@linux-foundation.org, jeffm@suse.com, netdev@vger.kernel.org, isdn4linux@koppen.de, isdn@linux-pingi.de, sergei.shtylyov@cogentembedded.com To: David Miller Return-path: Received: from mail-ee0-f52.google.com ([74.125.83.52]:53377 "EHLO mail-ee0-f52.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752449Ab3ITQOb (ORCPT ); Fri, 20 Sep 2013 12:14:31 -0400 Received: by mail-ee0-f52.google.com with SMTP id c41so376147eek.11 for ; Fri, 20 Sep 2013 09:14:30 -0700 (PDT) In-Reply-To: <20130920.115634.1931451843075283025.davem@davemloft.net> Sender: netdev-owner@vger.kernel.org List-ID: On 09/20/2013 05:56 PM, David Miller wrote: > From: Jiri Slaby > Date: Fri, 20 Sep 2013 15:44:33 +0200 > >> On 09/15/2013 01:28 AM, Ben Hutchings wrote: >>>> @@ -694,6 +699,10 @@ base_sock_ioctl(struct socket *sock, uns >>>> case IMSETDEVNAME: { struct mISDN_devrename dn; + if >>>> (!capable(CAP_SYS_ADMIN) && + !gid_eq(misdn_permitted_gid, >>>> current_gid()) && + !in_group_p(misdn_permitted_gid)) + >>>> return -EPERM; if (copy_from_user(&dn, (void __user *)arg, >>>> sizeof(dn))) { err = -EFAULT; >>> >>> This seems to be the important bit: renaming of devices (if allowed >>> at all) ought to be limited to CAP_SYS_ADMIN or possibly >>> CAP_NET_ADMIN. But why should the group that is allowed to use >>> mISDN data sockets also be allowed to do this? >> >> This is based on an old patch we are dragging in SUSE since 2009: >> http://www.isdn4linux.de/pipermail/isdn4linux/2009-December/004493.html >> https://bugzilla.novell.com/show_bug.cgi?id=564423 >> >> The whole point of the gid-based access was to still allow some user >> group to manipulate the device in an arbitrary way. >> >> So if everybody agrees I will just disallow rename to >> non-CAP_NET_ADMIN users and we are done? > > No we are not done, sorry. > > Having a device specific module parameter for this is wrong on several > fundamental levels. What I'm suggesting is just to put a !capable(CAP_NET_ADMIN) test into the rename path and nothing more. -- js suse labs