linux-kernel - Re: [PATCH net-next 2/5] netdev-genl: Dump napi_defer_hard

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: <20240830132808.33129d22@kernel.org>
Date: Fri, 30 Aug 2024 13:28:08 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Joe Damato <jdamato@...tly.com>
Cc: netdev@...r.kernel.org, edumazet@...gle.com, amritha.nambiar@...el.com,
 sridhar.samudrala@...el.com, sdf@...ichev.me, bjorn@...osinc.com,
 hch@...radead.org, willy@...radead.org, willemdebruijn.kernel@...il.com,
 skhawaja@...gle.com, Martin Karsten <mkarsten@...terloo.ca>, Donald Hunter
 <donald.hunter@...il.com>, "David S. Miller" <davem@...emloft.net>, Paolo
 Abeni <pabeni@...hat.com>, Jesper Dangaard Brouer <hawk@...nel.org>, Xuan
 Zhuo <xuanzhuo@...ux.alibaba.com>, Daniel Jurgens <danielj@...dia.com>,
 open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH net-next 2/5] netdev-genl: Dump napi_defer_hard_irqs

On Fri, 30 Aug 2024 10:10:47 +0100 Joe Damato wrote:
> > > +        name: defer-hard-irqs
> > > +        doc: The number of consecutive empty polls before IRQ deferral ends
> > > +             and hardware IRQs are re-enabled.
> > > +        type: s32  
> > 
> > Why is this a signed value? 🤔️  
> 
> In commit 6f8b12d661d0 ("net: napi: add hard irqs deferral
> feature"), napi_defer_hard_irqs was added to struct net_device as an
> int. I was trying to match that and thus made the field a signed int
> in the napi struct, as well.

It's probably because int is the default type in C.
The choice of types in netlink feels more deliberate.

> It looks like there was a possibility of overflow introduced in that
> commit in change_napi_defer_hard_irqs maybe ?
> 
> If you'd prefer I could:
>   - submit a Fixes to change the net_device field to a u32 and then
>     change the netlink code to also be u32
>   - add an overflow check (val > U32_MAX) in
>     change_napi_defer_hard_irqs
> 
> Which would mean for the v2 of this series:
>   - drop the overflow check I added in Patch 1
>   - Change netlink to use u32 in this patch 
> 
> What do you think?

Whether we want to clean things up internally is up to you, the overflow
check you're adding in sysfs seems good. We can use u32 in netlink, with
a check: max: s32-max and lift this requirement later if we ever need
the 32nd bit?