linux-kernel - Re: [PATCH v3 2/3] drivers: hv: vmbus: add test attributes to debugfs

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] [day] [month] [year] [list]

Date:   Wed, 21 Aug 2019 23:36:09 -0400
From:   Branden Bonaby <brandonbonaby94@...il.com>
To:     Michael Kelley <mikelley@...rosoft.com>
Cc:     KY Srinivasan <kys@...rosoft.com>,
        Haiyang Zhang <haiyangz@...rosoft.com>,
        Stephen Hemminger <sthemmin@...rosoft.com>,
        "sashal@...nel.org" <sashal@...nel.org>,
        "linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 2/3] drivers: hv: vmbus: add test attributes to debugfs

> > +What:           /sys/kernel/debug/hyperv/<UUID>/fuzz_test_state
> > +Date:           August 2019
> > +KernelVersion:  5.3
> > +Contact:        Branden Bonaby <brandonbonaby94@...il.com>
> > +Description:    Fuzz testing status of a vmbus device, whether its in an ON
> > +                state or a OFF state
> 
> Document what values are actually returned?  
> 
> > +Users:          Debugging tools
> > +
> > +What:           /sys/kernel/debug/hyperv/<UUID>/delay/fuzz_test_buffer_interrupt_delay
> > +Date:           August 2019
> > +KernelVersion:  5.3
> > +Contact:        Branden Bonaby <brandonbonaby94@...il.com>
> > +Description:    Fuzz testing buffer delay value between 0 - 1000
> 
> It would be helpful to document the units -- I think this is 0 to 1000
> microseconds.

you're right, that makes sense I'll add that information in. Also 
to confirm, it is microseconds like you said.

> > +static int hv_debugfs_delay_set(void *data, u64 val)
> > +{
> > +	if (val >= 1 && val <= 1000)
> > +		*(u32 *)data = val;
> > +	/*Best to not use else statement here since we want
> > +	 * the delay to remain the same if val > 1000
> > +	 */
> 
> The standard multi-line comment style would be:
> 
> 	/*
> 	 * Best to not use else statement here since we want
> 	 * the delay to remain the same if val > 1000
> 	 */
>

will change

> > +	else if (val <= 0)
> > +		*(u32 *)data = 0;
> 
> You could consider returning an error for an invalid
> value (< 0, or > 1000).
> 

its subtle but it does make sense and shows anyone
reading that the only acceptable values in the 
function are 0 <= 1000 at a glance. I'll add
that in.