From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754725AbbCLNtb (ORCPT ); Thu, 12 Mar 2015 09:49:31 -0400 Received: from mail.linuxfoundation.org ([140.211.169.12]:36465 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753174AbbCLNta (ORCPT ); Thu, 12 Mar 2015 09:49:30 -0400 Date: Thu, 12 Mar 2015 14:49:25 +0100 From: Greg Kroah-Hartman To: Guenter Roeck Cc: Vivien Didelot , linux-kernel@vger.kernel.org, kernel@savoirfairelinux.com Subject: Re: [PATCH v2 2/3] sysfs: Only accept read/write permissions for file attributes Message-ID: <20150312134925.GC32053@kroah.com> References: <1426098131-20106-1-git-send-email-vivien.didelot@savoirfairelinux.com> <1426098131-20106-3-git-send-email-vivien.didelot@savoirfairelinux.com> <20150312100151.GD3682@kroah.com> <55016CC8.3070304@roeck-us.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <55016CC8.3070304@roeck-us.net> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Mar 12, 2015 at 03:39:04AM -0700, Guenter Roeck wrote: > On 03/12/2015 03:01 AM, Greg Kroah-Hartman wrote: > >On Wed, Mar 11, 2015 at 02:22:10PM -0400, Vivien Didelot wrote: > >>For sysfs file attributes, only read and write permissions make sense. > >>Mask provided attribute permissions accordingly and send a warning > >>to the console if invalid permission bits are set. > >> > >>This patch is originally from Guenter [1] and includes the fixup > >>explained in the thread, that is printing permissions in octal format > >>and limiting the scope of attributes to SYSFS_PREALLOC | 0664. > >> > >>[1] https://lkml.org/lkml/2015/1/19/599 > >> > >>Cc: Guenter Roeck > >>Signed-off-by: Vivien Didelot > >>--- > >> fs/sysfs/group.c | 6 ++++++ > >> 1 file changed, 6 insertions(+) > >> > >>diff --git a/fs/sysfs/group.c b/fs/sysfs/group.c > >>index 3fdccd9..b400c04 100644 > >>--- a/fs/sysfs/group.c > >>+++ b/fs/sysfs/group.c > >>@@ -55,6 +55,12 @@ static int create_files(struct kernfs_node *parent, struct kobject *kobj, > >> if (!mode) > >> continue; > >> } > >>+ > >>+ WARN(mode & ~(SYSFS_PREALLOC | 0664), > >>+ "Attribute %s: Invalid permissions 0%o\n", > >>+ (*attr)->name, mode); > >>+ > >>+ mode &= SYSFS_PREALLOC | 0664; > > > >How does a "normal" boot look with this warning in place? There still > >seem to be a number of files in sysfs that might trigger this. > > > I was under the impression that they all were addressed, but I may have > missed some pattern(s). Can you point me to an example, by any chance ? Ah, nevermind, they seem to have all been fixed up. > >Also, we have a build-time warning if a sysfs file is this type of > >attribute, shouldn't we just rely on that instead of this run-time > >warning? > > > > The mode value can be returned from an is_visible function, and even if not > there is no guarantee that the build-time warning triggers (attribute lists > can be generated manually, for example). You are correct, sorry, I missed that, nevermind again. Once Vivien fixes up the signed-off-by stuff here, I'll be glad to queue them up. thanks, greg k-h