linux-kernel - Re: [PATCH v4 01/10] ufs: sysfs: attribute group for existing sysfs entries.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20180204123343.GA940@kroah.com>
Date:   Sun, 4 Feb 2018 13:33:43 +0100
From:   Greg KH <gregkh@...uxfoundation.org>
To:     Stanislav Nijnikov <Stanislav.Nijnikov@....com>
Cc:     "linux-scsi@...r.kernel.org" <linux-scsi@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "jaegeuk@...nel.org" <jaegeuk@...nel.org>,
        Alex Lemberg <Alex.Lemberg@....com>
Subject: Re: [PATCH v4 01/10] ufs: sysfs: attribute group for existing sysfs
 entries.

On Sun, Feb 04, 2018 at 12:29:06PM +0000, Stanislav Nijnikov wrote:
> > > +	curr_len += snprintf((buf + curr_len), (PAGE_SIZE - curr_len),
> > > +			     "\nAll available Runtime PM levels info:\n");
> > > +	for (lvl = UFS_PM_LVL_0; lvl < UFS_PM_LVL_MAX; lvl++)
> > > +		curr_len += snprintf((buf + curr_len), (PAGE_SIZE - curr_len),
> > > +				     "\tRuntime PM level [%d] => dev_state
> > [%s] link_state [%s]\n",
> > > +				    lvl,
> > > +				    ufschd_ufs_dev_pwr_mode_to_string(
> > > +					ufs_pm_lvl_states[lvl].dev_state),
> > > +				    ufschd_uic_link_state_to_string(
> > > +					ufs_pm_lvl_states[lvl].link_state));
> > > +
> > 
> > sysfs if "one value per file", not "random text that someone has to parse per
> > file" please.
> > 
> > Huge hint, if you ever care about checking the size of the sysfs buffer you are
> > writing into, you are doing something really really wrong.
> > 
> Hi Greg
> It's the existing code, added by:
> commit 09690d5a6ae1b7e4cb5ac429c311b99d09352c12
> Author: subhashj@...eaurora.org <subhashj@...eaurora.org>
> Date:   Thu Dec 22 18:41:00 2016 -0800
> 
>     scsi: ufs: provide sysfs attribute to select the PM level
>     
>     This patch provides the sysfs attribute to choose the power management
>     level for UFS runtime and system suspend.
>     
>     Reviewed-by: Sujit Reddy Thumma <sthumma@...eaurora.org>
>     Signed-off-by: Subhash Jadavani <subhashj@...eaurora.org>
>     Signed-off-by: Martin K. Petersen <martin.petersen@...cle.com>
> 
> I just moved it to an another file and changed the sysfs entries creation by
> Jaegeuk Kim' request. At the moment the entry shows the PM level, the device
> state, the link state and all possible PM levels. Do you want me to change it?

Ah, you are just moving this code around.  Ok, that's fine for this
patch, but please fix it up as part of this patch series because this
isn't an acceptable sysfs file at all.  If it were documented that would
be a lot more obvious as to just how wrong it was :(

And, as it wasn't documented, you can change it as it's obvious no one
used it :)

thanks,

greg k-h