Re: [PATCH v2 7/7] gpio: 74x164: Utilise temporary variable for struct device

2025-02-10 Thread Andy Shevchenko
On Fri, Feb 07, 2025 at 08:56:45PM +0100, Bartosz Golaszewski wrote:
> On Fri, Feb 7, 2025 at 4:42 PM Andy Shevchenko
>  wrote:
> >
> > On Fri, Feb 07, 2025 at 05:17:14PM +0200, Andy Shevchenko wrote:
> > > We have a temporary variable to keep a pointer to struct device.
> > > Utilise it where it makes sense.
> >
> > Urgh, this seems incomplete...
> > There are more lines to convert, however they do not affect ą statistics.
> >
> > Tell me if I need to send full v3 or just this patch.
> 
> No worries, I may fix it when applying.
> 
> I'll still give this series a few more days on the list.

Sure, thanks!


-- 
With Best Regards,
Andy Shevchenko





Re: [PATCH v1 2/7] gpio: 74x164: Simplify code with cleanup helpers

2025-02-10 Thread Linus Walleij
On Mon, Feb 3, 2025 at 1:18 PM Andy Shevchenko
 wrote:

> Use macros defined in linux/cleanup.h to automate resource lifetime
> control in the driver.
>
> Signed-off-by: Andy Shevchenko 

Clearly this is more readable and less prone to accidental bugs.
Reviewed-by: Linus Walleij 

Yours,
Linus Walleij



Re: [PATCH v1 4/7] gpio: 74x164: Make use of the macros from bits.h

2025-02-10 Thread Linus Walleij
On Mon, Feb 3, 2025 at 1:18 PM Andy Shevchenko
 wrote:

> Make use of BIT() and GENMASK() where it makes sense.
>
> Signed-off-by: Andy Shevchenko 

Reviewed-by: Linus Walleij 

Yours,
Linus Walleij



Re: [PATCH v1 4/7] gpio: 74x164: Make use of the macros from bits.h

2025-02-10 Thread Andy Shevchenko
On Mon, Feb 10, 2025 at 10:12:52AM +0100, Linus Walleij wrote:
> On Mon, Feb 3, 2025 at 1:18 PM Andy Shevchenko
>  wrote:
> 
> > Make use of BIT() and GENMASK() where it makes sense.
> >
> > Signed-off-by: Andy Shevchenko 
> 
> Reviewed-by: Linus Walleij 

Thanks, there is a v2 available.

-- 
With Best Regards,
Andy Shevchenko





Re: [PATCH v2 1/2] slab: Adjust placement of __kvmalloc_node_noprof

2025-02-10 Thread Vlastimil Babka
On 2/8/25 02:47, GONG Ruiqi wrote:
> Move __kvmalloc_node_noprof (and also kvfree* for consistency) into
> mm/slub.c so that it can directly invoke __do_kmalloc_node, which is
> needed for the next patch. Move kmalloc_gfp_adjust to slab.h since now
> its two callers are in different .c files.
> 
> No functional changes intended.
> 
> Signed-off-by: GONG Ruiqi 
> ---
>  include/linux/slab.h |  22 +
>  mm/slub.c|  90 ++
>  mm/util.c| 112 ---
>  3 files changed, 112 insertions(+), 112 deletions(-)
> 
> diff --git a/include/linux/slab.h b/include/linux/slab.h
> index 09eedaecf120..0bf4cbf306fe 100644
> --- a/include/linux/slab.h
> +++ b/include/linux/slab.h

This could be mm/slab.h instead.
But I'd just go all the way and move kvrealloc_noprof() too, it would be
more consistent anyway.




Re: [PATCH v2 2/2] slab: Achieve better kmalloc caches randomization in kvmalloc

2025-02-10 Thread Vlastimil Babka
On 2/8/25 02:47, GONG Ruiqi wrote:
> As revealed by this writeup[1], due to the fact that __kmalloc_node (now
> renamed to __kmalloc_node_noprof) is an exported symbol and will never
> get inlined, using it in kvmalloc_node (now is __kvmalloc_node_noprof)
> would make the RET_IP inside always point to the same address:
> 
> upper_caller
> kvmalloc
> kvmalloc_node
> kvmalloc_node_noprof
> __kvmalloc_node_noprof<-- all macros all the way down here
> __kmalloc_node_noprof
> __do_kmalloc_node(.., _RET_IP_)
> ...   <-- _RET_IP_ points to
> 
> That literally means all kmalloc invoked via kvmalloc would use the same
> seed for cache randomization (CONFIG_RANDOM_KMALLOC_CACHES), which makes
> this hardening unfunctional.

 non-functional?

> The root cause of this problem, IMHO, is that using RET_IP only cannot
> identify the actual allocation site in case of kmalloc being called
> inside wrappers or helper functions.

inside non-inlined wrappers... ?

>  And I believe there could be
> similar cases in other functions. Nevertheless, I haven't thought of
> any good solution for this. So for now let's solve this specific case
> first.

Yeah it's the similar problem with shared allocation wrappers as what
allocation tagging has.

> For __kvmalloc_node_noprof, replace __kmalloc_node_noprof and call
> __do_kmalloc_node directly instead, so that RET_IP can take the return
> address of kvmalloc and differentiate each kvmalloc invocation:
> 
> upper_caller
> kvmalloc
> kvmalloc_node
> kvmalloc_node_noprof
> __kvmalloc_node_noprof<-- all macros all the way down here
> __do_kmalloc_node(.., _RET_IP_)
> ...   <-- _RET_IP_ points to
> 
> Thanks to Tamás Koczka for the report and discussion!
> 
> Link: 
> https://github.com/google/security-research/pull/83/files#diff-1604319b55a48c39a210ee52034ed7ff5b9cdc3d704d2d9e34eb230d19fae235R200
>  [1]

This should be slightly better? A permalink for the file itself:
https://github.com/google/security-research/blob/908d59b573960dc0b90adda6f16f7017aca08609/pocs/linux/kernelctf/CVE-2024-27397_mitigation/docs/exploit.md

Thanks.

> Reported-by: Tamás Koczka 
> Signed-off-by: GONG Ruiqi 
> ---
>  mm/slub.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/mm/slub.c b/mm/slub.c
> index 0830894bb92c..46e884b77dca 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -4903,9 +4903,9 @@ void *__kvmalloc_node_noprof(DECL_BUCKET_PARAMS(size, 
> b), gfp_t flags, int node)
>* It doesn't really make sense to fallback to vmalloc for sub page
>* requests
>*/
> - ret = __kmalloc_node_noprof(PASS_BUCKET_PARAMS(size, b),
> - kmalloc_gfp_adjust(flags, size),
> - node);
> + ret = __do_kmalloc_node(size, PASS_BUCKET_PARAM(b),
> + kmalloc_gfp_adjust(flags, size),
> + node, _RET_IP_);
>   if (ret || size <= PAGE_SIZE)
>   return ret;
>  




Re: [RFC PATCH v3 00/15] pkeys-based page table hardening

2025-02-10 Thread Kevin Brodsky
On 06/02/2025 23:41, Kees Cook wrote:
> On Mon, Feb 03, 2025 at 10:18:24AM +, Kevin Brodsky wrote:
>> This is a proposal to leverage protection keys (pkeys) to harden
>> critical kernel data, by making it mostly read-only. The series includes
>> a simple framework called "kpkeys" to manipulate pkeys for in-kernel use,
>> as well as a page table hardening feature based on that framework
>> (kpkeys_hardened_pgtables). Both are implemented on arm64 as a proof of
>> concept, but they are designed to be compatible with any architecture
>> implementing pkeys.
> Does QEMU support POE? The only mention I could find is here:
> https://mail.gnu.org/archive/html/qemu-arm/2024-03/msg00486.html
> where the answer is, "no and it looks difficult". :P

Unfortunately it looks like the answer hasn't changed since last year. I
am testing this series on an Arm Fast Models platform (FVP) [1], which
does support POE. I've included instructions to get you started at the end.

>> # Threat model
>>
>> The proposed scheme aims at mitigating data-only attacks (e.g.
>> use-after-free/cross-cache attacks). In other words, it is assumed that
>> control flow is not corrupted, and that the attacker does not achieve
>> arbitrary code execution. Nothing prevents the pkey register from being
>> set to its most permissive state - the assumption is that the register
>> is only modified on legitimate code paths.
> Do you have any tests that could be added to drivers/misc/lkdtm that
> explicitly exercise the protection? That is where many hardware security
> features get tested. (i.e. a successful test will generally trigger a
> BUG_ON or similar.)

I could certainly add some tests there, but I wonder if such crash tests
provide much benefit compared to the KUnit tests (that rely on
copy_to_kernel_nofault()) in patch 15? Not crashing the kernel does mean
that many of those tests can be run in a row :)

>> The arm64 implementation should be considered a proof of concept only.
>> The enablement of POE for in-kernel use is incomplete; in particular
>> POR_EL1 (pkey register) should be reset on exception entry and restored
>> on exception return.
> As in, make sure the loaded pkey isn't leaked into an exception handler?

I wouldn't say "leaking" is the issue here, but yes conceptually
exception handlers should run with a fixed pkey configuration, not that
of the interrupted context. As Dave Hansen pointed out [2], what is even
more important is to context-switch the pkey register. A thread may be
interrupted and scheduled out while executing at a higher kpkeys level;
we want to ensure that this thread resumes execution at the same kpkeys
level, and that in the meantime we return to the standard level.

>> # Open questions
>>
>> A few aspects in this RFC that are debatable and/or worth discussing:
>>
>> - There is currently no restriction on how kpkeys levels map to pkeys
>>   permissions. A typical approach is to allocate one pkey per level and
>>   make it writable at that level only. As the number of levels
>>   increases, we may however run out of pkeys, especially on arm64 (just
>>   8 pkeys with POE). Depending on the use-cases, it may be acceptable to
>>   use the same pkey for the data associated to multiple levels.
>>
>>   Another potential concern is that a given piece of code may require
>>   write access to multiple privileged pkeys. This could be addressed by
>>   introducing a notion of hierarchy in trust levels, where Tn is able to
>>   write to memory owned by Tm if n >= m, for instance.
>>
>> - kpkeys_set_level() and kpkeys_restore_pkey_reg() are not symmetric:
>>   the former takes a kpkeys level and returns a pkey register value, to
>>   be consumed by the latter. It would be more intuitive to manipulate
>>   kpkeys levels only. However this assumes that there is a 1:1 mapping
>>   between kpkeys levels and pkey register values, while in principle
>>   the mapping is 1:n (certain pkeys may be used outside the kpkeys
>>   framework).
> Is the "levels" nature of this related to how POE behaves? It sounds
> like there can only be 1 pkey active at a time (a role), rather than
> each pkey representing access to a specific set of pages (a key in a
> keyring), where many pkeys could be active at the same time. Am I
> understanding that correctly?

Only one key is used (besides the default key) in this initial RFC.
However, the idea behind the level abstraction is indeed that (RW)
access to multiple keys may be required at the same time. In the
follow-up RFC protecting credentials, this is illustrated by the
"unrestricted" level that grants RW access to all keys. I believe this
approach is the most flexible, in that any permission mapping can be
defined for each level.

>> Any comment or feedback will be highly appreciated, be it on the
>> high-level approach or implementation choices!
> As hinted earlier with my QEMU question... what's the best way I can I
> test this myself? :)

As mentioned above I tested this series on Arm FVP. By

Re: [PATCH] ixgbe: remove self assignment

2025-02-10 Thread Andrew Lunn
On Sun, Feb 09, 2025 at 11:47:24PM -0500, Ethan Carter Edwards wrote:
> Variable self assignment does not have any effect.

Hi Ethan

As a general rule, it would be good to explain in the comment message
what research you did to find out why there is a self assignment, and
why just deleting it is the correct solution.

There are somewhat legitimate reasons to do a self assign, some older
compilers would warn about variables which were set but then never
used, for example. Or it could be a dumb copy/paste error when writing
the code. But more likely than not, the developer had something in
mind, got distracted, and never finished the code. Which appears to
the issue here.

If you cannot figure out what the correct fix is, please just email to
the list, Cc: the Maintainer of the file, pointing out the problem.

Andrew

---
pw-bot: cr



Re: [PATCH v4 22/30] context_tracking: Exit CT_STATE_IDLE upon irq/nmi entry

2025-02-10 Thread Valentin Schneider
On 07/02/25 19:37, Frederic Weisbecker wrote:
> Le Fri, Feb 07, 2025 at 06:06:45PM +0100, Valentin Schneider a écrit :
>>
>> S I've been thinking...
>>
>> Isn't
>>
>>   (context_tracking.state & CT_RCU_WATCHING)
>>
>> pretty much a proxy for knowing whether a CPU is executing in kernelspace,
>> including NMIs?
>
> You got it!
>

Yay!

>>
>> NMI interrupts userspace/VM/idle -> ct_nmi_enter()   -> it becomes true
>> IRQ interrupts idle  -> ct_irq_enter()   -> it becomes true
>> IRQ interrupts userspace -> __ct_user_exit() -> it becomes true
>> IRQ interrupts VM-> __ct_user_exit() -> it becomes true
>>
>> IOW, if I gate setting deferred work by checking for this instead of
>> explicitely CT_STATE_KERNEL, "it should work" and prevent the
>> aforementioned issue? Or should I be out drinking instead? :-)
>
> Exactly it should work! Now that doesn't mean you can't go out
> for a drink :-)
>

Well, drinks were had very shortly after sending this email :D

> Thanks.




[PATCH] net/mlx4_core: Avoid impossible mlx4_db_alloc() order value

2025-02-10 Thread Kees Cook
GCC can see that the value range for "order" is capped, but this leads
it to consider that it might be negative, leading to a false positive
warning (with GCC 15 with -Warray-bounds -fdiagnostics-details):

../drivers/net/ethernet/mellanox/mlx4/alloc.c:691:47: error: array subscript -1 
is below array bounds of 'long unsigned int *[2]' [-Werror=array-bounds=]
  691 | i = find_first_bit(pgdir->bits[o], MLX4_DB_PER_PAGE >> 
o);
  |~~~^~~
  'mlx4_alloc_db_from_pgdir': events 1-2
  691 | i = find_first_bit(pgdir->bits[o], MLX4_DB_PER_PAGE >> 
o);| 
^
  | | | 
  | | (2) 
out of array bounds here
  | (1) when the condition is evaluated to true 
In file included from 
../drivers/net/ethernet/mellanox/mlx4/mlx4.h:53,
 from ../drivers/net/ethernet/mellanox/mlx4/alloc.c:42:
../include/linux/mlx4/device.h:664:33: note: while referencing 'bits'
  664 | unsigned long  *bits[2];
  | ^~~~

Switch the argument to unsigned int, which removes the compiler needing
to consider negative values.

Signed-off-by: Kees Cook 
---
Cc: Tariq Toukan 
Cc: Andrew Lunn 
Cc: "David S. Miller" 
Cc: Eric Dumazet 
Cc: Jakub Kicinski 
Cc: Paolo Abeni 
Cc: Yishai Hadas 
Cc: net...@vger.kernel.org
Cc: linux-r...@vger.kernel.org
---
 drivers/net/ethernet/mellanox/mlx4/alloc.c | 6 +++---
 include/linux/mlx4/device.h| 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/alloc.c 
b/drivers/net/ethernet/mellanox/mlx4/alloc.c
index b330020dc0d6..f2bded847e61 100644
--- a/drivers/net/ethernet/mellanox/mlx4/alloc.c
+++ b/drivers/net/ethernet/mellanox/mlx4/alloc.c
@@ -682,9 +682,9 @@ static struct mlx4_db_pgdir *mlx4_alloc_db_pgdir(struct 
device *dma_device)
 }
 
 static int mlx4_alloc_db_from_pgdir(struct mlx4_db_pgdir *pgdir,
-   struct mlx4_db *db, int order)
+   struct mlx4_db *db, unsigned int order)
 {
-   int o;
+   unsigned int o;
int i;
 
for (o = order; o <= 1; ++o) {
@@ -712,7 +712,7 @@ static int mlx4_alloc_db_from_pgdir(struct mlx4_db_pgdir 
*pgdir,
return 0;
 }
 
-int mlx4_db_alloc(struct mlx4_dev *dev, struct mlx4_db *db, int order)
+int mlx4_db_alloc(struct mlx4_dev *dev, struct mlx4_db *db, unsigned int order)
 {
struct mlx4_priv *priv = mlx4_priv(dev);
struct mlx4_db_pgdir *pgdir;
diff --git a/include/linux/mlx4/device.h b/include/linux/mlx4/device.h
index 27f42f713c89..86f0f2a25a3d 100644
--- a/include/linux/mlx4/device.h
+++ b/include/linux/mlx4/device.h
@@ -1135,7 +1135,7 @@ int mlx4_write_mtt(struct mlx4_dev *dev, struct mlx4_mtt 
*mtt,
 int mlx4_buf_write_mtt(struct mlx4_dev *dev, struct mlx4_mtt *mtt,
   struct mlx4_buf *buf);
 
-int mlx4_db_alloc(struct mlx4_dev *dev, struct mlx4_db *db, int order);
+int mlx4_db_alloc(struct mlx4_dev *dev, struct mlx4_db *db, unsigned int 
order);
 void mlx4_db_free(struct mlx4_dev *dev, struct mlx4_db *db);
 
 int mlx4_alloc_hwq_res(struct mlx4_dev *dev, struct mlx4_hwq_resources *wqres,
-- 
2.34.1




Re: [PATCH 06/10] x86/tdx: Mark message.str as nonstring

2025-02-10 Thread Kirill A. Shutemov
On Thu, Feb 06, 2025 at 06:37:27PM -0800, Kees Cook wrote:
> On Thu, Feb 06, 2025 at 05:12:11PM -0800, Dave Hansen wrote:
> > On 2/6/25 17:00, Kees Cook wrote:
> > > +++ b/arch/x86/coco/tdx/tdx.c
> > > @@ -170,7 +170,7 @@ static void __noreturn tdx_panic(const char *msg)
> > >   /* Define register order according to the GHCI */
> > >   struct { u64 r14, r15, rbx, rdi, rsi, r8, r9, rdx; };
> > >  
> > > - char str[64];
> > > + char str[64] __nonstring;
> > >   } message;
> > 
> > So, the patch itself makes sense. But it does end up looking kinda
> > funky. We call it a "str"ing and then annotate it as not a string.
> 
> Yeah, this is true all over the place. It's a string, just not a
> NUL-terminated string: *sob*
> 
> > It doesn't have to be done in this patch, but it does seem like we
> > should probably not be using 'char' and also shouldn't call it anything
> > close to "string". Maybe:
> > 
> > u8 message[64] __nonstring;
> > } message;
> 
> message.message ;)
> 
> message.chars?
> message.bytes?

.bytes sounds good to me.

Anyway:

Acked-by: Kirill A. Shutemov 

-- 
  Kiryl Shutsemau / Kirill A. Shutemov



Re: [PATCH v4 29/30] x86/mm, mm/vmalloc: Defer flush_tlb_kernel_range() targeting NOHZ_FULL CPUs

2025-02-10 Thread Valentin Schneider
On 17/01/25 16:52, Jann Horn wrote:
> On Fri, Jan 17, 2025 at 4:25 PM Valentin Schneider  
> wrote:
>> On 14/01/25 19:16, Jann Horn wrote:
>> > On Tue, Jan 14, 2025 at 6:51 PM Valentin Schneider  
>> > wrote:
>> >> vunmap()'s issued from housekeeping CPUs are a relatively common source of
>> >> interference for isolated NOHZ_FULL CPUs, as they are hit by the
>> >> flush_tlb_kernel_range() IPIs.
>> >>
>> >> Given that CPUs executing in userspace do not access data in the vmalloc
>> >> range, these IPIs could be deferred until their next kernel entry.
>> >>
>> >> Deferral vs early entry danger zone
>> >> ===
>> >>
>> >> This requires a guarantee that nothing in the vmalloc range can be 
>> >> vunmap'd
>> >> and then accessed in early entry code.
>> >
>> > In other words, it needs a guarantee that no vmalloc allocations that
>> > have been created in the vmalloc region while the CPU was idle can
>> > then be accessed during early entry, right?
>>
>> I'm not sure if that would be a problem (not an mm expert, please do
>> correct me) - looking at vmap_pages_range(), flush_cache_vmap() isn't
>> deferred anyway.
>
> flush_cache_vmap() is about stuff like flushing data caches on
> architectures with virtually indexed caches; that doesn't do TLB
> maintenance. When you look for its definition on x86 or arm64, you'll
> see that they use the generic implementation which is simply an empty
> inline function.
>
>> So after vmapping something, I wouldn't expect isolated CPUs to have
>> invalid TLB entries for the newly vmapped page.
>>
>> However, upon vunmap'ing something, the TLB flush is deferred, and thus
>> stale TLB entries can and will remain on isolated CPUs, up until they
>> execute the deferred flush themselves (IOW for the entire duration of the
>> "danger zone").
>>
>> Does that make sense?
>
> The design idea wrt TLB flushes in the vmap code is that you don't do
> TLB flushes when you unmap stuff or when you map stuff, because doing
> TLB flushes across the entire system on every vmap/vunmap would be a
> bit costly; instead you just do batched TLB flushes in between, in
> __purge_vmap_area_lazy().
>
> In other words, the basic idea is that you can keep calling vmap() and
> vunmap() a bunch of times without ever doing TLB flushes until you run
> out of virtual memory in the vmap region; then you do one big TLB
> flush, and afterwards you can reuse the free virtual address space for
> new allocations again.
>
> So if you "defer" that batched TLB flush for CPUs that are not
> currently running in the kernel, I think the consequence is that those
> CPUs may end up with incoherent TLB state after a reallocation of the
> virtual address space.
>
> Actually, I think this would mean that your optimization is disallowed
> at least on arm64 - I'm not sure about the exact wording, but arm64
> has a "break before make" rule that forbids conflicting writable
> address translations or something like that.
>
> (I said "until you run out of virtual memory in the vmap region", but
> that's not actually true - see the comment above lazy_max_pages() for
> an explanation of the actual heuristic. You might be able to tune that
> a bit if you'd be significantly happier with less frequent
> interruptions, or something along those lines.)

I've been thinking some more (this is your cue to grab a brown paper bag)...

Experimentation (unmap the whole VMALLOC range upon return to userspace,
see what explodes upon entry into the kernel) suggests that the early entry
"danger zone" should only access the vmaped stack, which itself isn't an
issue.

That is obviously just a test on one system configuration, and the problem
I'm facing is trying put in place /some/ form of instrumentation that would
at the very least cause a warning for any future patch that would introduce
a vmap'd access in early entry code. That, or a complete mitigation that
prevents those accesses altogether.

What if isolated CPUs unconditionally did a TLBi as late as possible in
the stack right before returning to userspace? This would mean that upon
re-entering the kernel, an isolated CPU's TLB wouldn't contain any kernel
range translation - with the exception of whatever lies between the
last-minute flush and the actual userspace entry, which should be feasible
to vet? Then AFAICT there wouldn't be any work/flush to defer, the IPI
could be entirely silenced if it targets an isolated CPU.




Re: [RFC PATCH v1 1/1] selftest/mm: refactor mseal_test

2025-02-10 Thread Jeff Xu
Hi Lorenzo,

Gentle ping for my clarification questions.

I also tried the new ioctl PROCMAP_QUERY, please see below for details.

On Wed, Jan 15, 2025 at 12:47 PM Jeff Xu  wrote:
>
> Hi Lorenzo,
>
> On Thu, Jan 2, 2025 at 9:30 AM Lorenzo Stoakes
>  wrote:
> >
> > Sorry for delay, was super busy leading up to xmas break, then had ~2.5
> > weeks off.
> >
> Thanks for reviewing.  There are lots of comments, so it takes some
> time to go through comments and experiment with some of the suggested
> changes. Please see below.
>
> > And happy new year! :)
> >
> Happy new year !
>
> > On Wed, Dec 11, 2024 at 05:33:11AM +, jef...@chromium.org wrote:
> > > From: Jeff Xu 
> > >
> > > This change creates the initial version of memorysealing.c.
> > >
> > > The introduction of memorysealing.c, which replaces mseal_test.c and
> >
> > I mean I don't want to be a pain but I would kinda prefer to have 'mseal'
> > everywhere mseal is to avoid confusion vs. memfd seals.
> >
> As I tried to explain, the memorysealing.c will eventually replace
> mseal_test.c, after all the tests are moved to use kselftest_karness.
> How about mseal_test_new.c ? I'm open to new names if you have one.
>
> > Of course we in the kernel absolutely _love_ to overload the meaning of 
> > terms
> > but some traditions are worth breaking :)
> >
> > > uses the kselftest_harness, aims to initiate a discussion on using the
> > > selftest harness for memory sealing tests. Upon approval of this
> > > approach, the migration of tests from mseal_test.c to memorysealing.c
> > > can be implemented in a step-by-step manner.
> >
> > Well, I for one like this approach so (unsurprisingly) :) but perhaps
> > sensible to do it iteratively so we can also tweak things as we go.
> >
> Yes, that is the idea.
>
> > >
> > > This tests addresses following feedback from previous reviews:
> > >
> > > 1> Use kselftest_harness instead of custom macro, such as EXPECT_XX,
> > > ASSERT_XX, etc.  (Lorenzo Stoakes, Mark Brown) [1]
> > >
> > > 2> Use MAP_FAILED to check the return of mmap (Lorenzo Stoakes).
> >
> > Thanks!
> >
> > >
> > > 3>  Adding a check for vma size and prot bits. The discussion for
> > > this can be found in [2] [3], here is a brief summary:
> > > This is to follow up on Pedro’s in-loop change (from
> > > can_modify_mm to can_modify_vma). When mseal_test is initially
> > > created, they have a common pattern:  setup memory layout,
> > > seal the memory, perform a few mm-api steps, verify return code
> > > (not zero).  Because of the nature of out-of-loop,  it is sufficient
> > > to just verify the error code in a few cases.
> > >
> > > With Pedro's in-loop change, the sealing check happens later in the
> > > stack, thus there are more things and scenarios to verify. And there
> > > was feedbacks to me that mseal_test should be extensive enough to
> > > discover all regressions. Hence I'm adding check for vma size and prot
> > > bits.
> > >
> > > In this change: we created two fixtures:
> > >
> > > Fixture basic:   This creates a single VMA, the VMA has a
> > > PROT_NONE page at each end to prevent auto-merging.
> > >
> > > Fixture wo_vma: Two VMAs back to end, a PROT_NONE page at each
> > > end to prevent auto-merging.
> > >
> > > In addition, I add one test (mprotec) in each fixture to demo the tests.
> > >
> > > [1] 
> > > https://lore.kernel.org/all/20240830180237.1220027-5-jef...@chromium.org/
> > > [2] 
> > > https://lore.kernel.org/all/CABi2SkUgDZtJtRJe+J9UNdtZn=EQzZcbMB685P=1rR7DUhg=6...@mail.gmail.com/
> > > [3] 
> > > https://lore.kernel.org/all/2qywbjb5ebtgwkh354w3lj3vhaothvubjokxq5fhyri5jeeton@duqngzo3swjz/
> > >
> > > Signed-off-by: Jeff Xu 
> > > ---
> > >  tools/testing/selftests/mm/.gitignore  |   1 +
> > >  tools/testing/selftests/mm/Makefile|   1 +
> > >  tools/testing/selftests/mm/memorysealing.c | 182 +
> > >  tools/testing/selftests/mm/memorysealing.h | 116 +
> > >  tools/testing/selftests/mm/mseal_test.c|  67 +---
> > >  5 files changed, 301 insertions(+), 66 deletions(-)
> > >  create mode 100644 tools/testing/selftests/mm/memorysealing.c
> > >  create mode 100644 tools/testing/selftests/mm/memorysealing.h
> > >
> > > diff --git a/tools/testing/selftests/mm/.gitignore 
> > > b/tools/testing/selftests/mm/.gitignore
> > > index a51a947b2d1d..982234a99f20 100644
> > > --- a/tools/testing/selftests/mm/.gitignore
> > > +++ b/tools/testing/selftests/mm/.gitignore
> > > @@ -57,3 +57,4 @@ hugetlb_dio
> > >  pkey_sighandler_tests_32
> > >  pkey_sighandler_tests_64
> > >  guard-pages
> > > +memorysealing
> > > diff --git a/tools/testing/selftests/mm/Makefile 
> > > b/tools/testing/selftests/mm/Makefile
> > > index 93a46ac633df..08876624f46d 100644
> > > --- a/tools/testing/selftests/mm/Makefile
> > > +++ b/tools/testing/selftests/mm/Makefile
> > > @@ -67,6 +67,7 @@ TEST_GEN_FILES += map_populate
> > >  ifneq (,$(filter $(ARCH),arm64 riscv riscv64 x

Re: [PATCH] wifi: ath12k: Fix uninitialized variable and remove goto

2025-02-10 Thread Ethan Carter Edwards
On 25/02/10 05:08PM, Jeff Johnson wrote:
> On 2/9/2025 8:36 PM, Ethan Carter Edwards wrote:
> 
> commit subject should be as specific as possible.
> suggest you at least mention the function
Will change. Thanks!

> 
> > There is a possibility for an uninitialized *ret* variable to be
> > returned in some code paths.
> > 
> > This moves *ret* inside the conditional and explicitly returns 0 without
> > an error. Also removes goto that returned *ret*.
> 
> ath driver convention is to declare function variables at the beginning of the
> function -- please do not relocate the definition of 'ret'
Will fix.

> 
> > 
> > Addresses-Coverity-ID: 1642337 ("Uninitialized scalar variable")
> 
> Is that an official kernel tag? IMO the proper tag would be
So, it isn't "official" as far as I can tell, but it is widely used in
other commits, especially by Gustavo Silva. 

Also: 
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=778e2478d19574508861bcb1806d6e34d095587c

Coverity-IDs: is another option I have found. I have seen Closes: a few
times as well. I'm not really sure what the best option is, honestly.

I'll use Closes: in v2. LMK if you want me to use something else.

> 
> Closes: 
> https://scan5.scan.coverity.com/#/project-view/63541/10063?selectedIssue=1642337
> 
> (is there a shorter URL?)
Not that I know of.

> 
> see 
> https://www.kernel.org/doc/html/latest/process/submitting-patches.html#using-reported-by-tested-by-reviewed-by-suggested-by-and-fixes
> 
> > Fixes: b716a10d99a28 ("wifi: ath12k: enable MLO setup and teardown from 
> > core")
> > Signed-off-by: Ethan Carter Edwards 
> > ---
> >  drivers/net/wireless/ath/ath12k/core.c | 8 +++-
> >  1 file changed, 3 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/net/wireless/ath/ath12k/core.c 
> > b/drivers/net/wireless/ath/ath12k/core.c
> > index 
> > 0606116d6b9c491b6ede401b2e1aedfb619339a8..ae75cdad3bd6b6eb80a35fee94c18d365d123cbd
> >  100644
> > --- a/drivers/net/wireless/ath/ath12k/core.c
> > +++ b/drivers/net/wireless/ath/ath12k/core.c
> > @@ -908,7 +908,6 @@ int ath12k_mac_mlo_ready(struct ath12k_hw_group *ag)
> >  {
> > struct ath12k_hw *ah;
> > struct ath12k *ar;
> > -   int ret;
> > int i, j;
> >  
> > for (i = 0; i < ag->num_hw; i++) {
> > @@ -918,14 +917,13 @@ int ath12k_mac_mlo_ready(struct ath12k_hw_group *ag)
> >  
> > for_each_ar(ah, ar, j) {
> > ar = &ah->radio[j];
> > -   ret = __ath12k_mac_mlo_ready(ar);
> > +   int ret = __ath12k_mac_mlo_ready(ar);
> > if (ret)
> > -   goto out;
> > +   return ret;
> > }
> > }
> >  
> > -out:
> > -   return ret;
> > +   return 0;
> >  }
> >  
> >  static int ath12k_core_mlo_setup(struct ath12k_hw_group *ag)
> > 
> > ---
> > base-commit: a64dcfb451e254085a7daee5fe51bf22959d52d3
> > change-id: 20250209-ath12k-uninit-18560fd91c07
> > 
> > Best regards,
> 
> replacing goto out with return ret and unconditional return 0 LGTM.
> 
> can you respin a v2 with the other comments addressed?
Will do! Thank ya much.

> 
> /jeff



[PATCH v2] wifi: ath12k: cleanup ath12k_mac_mlo_ready()

2025-02-10 Thread Ethan Carter Edwards
There is a possibility for an uninitialized *ret* variable to be
returned in some code paths.

This explicitly returns 0 without an error. Also removes goto that
returned *ret* and simply returns in place.

Closes: 
https://scan5.scan.coverity.com/#/project-view/63541/10063?selectedIssue=1642337
Fixes: b716a10d99a28 ("wifi: ath12k: enable MLO setup and teardown from core")
Signed-off-by: Ethan Carter Edwards 
---
Changes in v2:
- Rewrite commit subject to include function
- put int ret; back at top of function declaration
- minor changes to description
- Link to v1: 
https://lore.kernel.org/r/20250209-ath12k-uninit-v1-1-afc800584...@ethancedwards.com
---
 drivers/net/wireless/ath/ath12k/core.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ath/ath12k/core.c 
b/drivers/net/wireless/ath/ath12k/core.c
index 
0606116d6b9c491b6ede401b2e1aedfb619339a8..276f2ee553f4e3529abff14f2b9238ee0a78f45e
 100644
--- a/drivers/net/wireless/ath/ath12k/core.c
+++ b/drivers/net/wireless/ath/ath12k/core.c
@@ -920,12 +920,11 @@ int ath12k_mac_mlo_ready(struct ath12k_hw_group *ag)
ar = &ah->radio[j];
ret = __ath12k_mac_mlo_ready(ar);
if (ret)
-   goto out;
+   return ret;
}
}
 
-out:
-   return ret;
+   return 0;
 }
 
 static int ath12k_core_mlo_setup(struct ath12k_hw_group *ag)

---
base-commit: a64dcfb451e254085a7daee5fe51bf22959d52d3
change-id: 20250209-ath12k-uninit-18560fd91c07

Best regards,
-- 
Ethan Carter Edwards 




Re: [PATCH] net/mlx4_core: Avoid impossible mlx4_db_alloc() order value

2025-02-10 Thread Justin Stitt
On Mon, Feb 10, 2025 at 09:45:05AM -0800, Kees Cook wrote:
> GCC can see that the value range for "order" is capped, but this leads
> it to consider that it might be negative, leading to a false positive
> warning (with GCC 15 with -Warray-bounds -fdiagnostics-details):
> 
> ../drivers/net/ethernet/mellanox/mlx4/alloc.c:691:47: error: array subscript 
> -1 is below array bounds of 'long unsigned int *[2]' [-Werror=array-bounds=]
>   691 | i = find_first_bit(pgdir->bits[o], MLX4_DB_PER_PAGE 
> >> o);
>   |~~~^~~
>   'mlx4_alloc_db_from_pgdir': events 1-2
>   691 | i = find_first_bit(pgdir->bits[o], MLX4_DB_PER_PAGE 
> >> o);| 
> ^
>   | | |   
> | | 
> (2) out of array bounds here
>   | (1) when the condition is evaluated to true   
>   In file included from 
> ../drivers/net/ethernet/mellanox/mlx4/mlx4.h:53,
>  from ../drivers/net/ethernet/mellanox/mlx4/alloc.c:42:
> ../include/linux/mlx4/device.h:664:33: note: while referencing 'bits'
>   664 | unsigned long  *bits[2];
>   | ^~~~
> 
> Switch the argument to unsigned int, which removes the compiler needing
> to consider negative values.
> 
> Signed-off-by: Kees Cook 
> ---
> Cc: Tariq Toukan 
> Cc: Andrew Lunn 
> Cc: "David S. Miller" 
> Cc: Eric Dumazet 
> Cc: Jakub Kicinski 
> Cc: Paolo Abeni 
> Cc: Yishai Hadas 
> Cc: net...@vger.kernel.org
> Cc: linux-r...@vger.kernel.org
> ---
>  drivers/net/ethernet/mellanox/mlx4/alloc.c | 6 +++---
>  include/linux/mlx4/device.h| 2 +-
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx4/alloc.c 
> b/drivers/net/ethernet/mellanox/mlx4/alloc.c
> index b330020dc0d6..f2bded847e61 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/alloc.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/alloc.c
> @@ -682,9 +682,9 @@ static struct mlx4_db_pgdir *mlx4_alloc_db_pgdir(struct 
> device *dma_device)
>  }
>  
>  static int mlx4_alloc_db_from_pgdir(struct mlx4_db_pgdir *pgdir,
> - struct mlx4_db *db, int order)
> + struct mlx4_db *db, unsigned int order)
>  {
> - int o;
> + unsigned int o;
>   int i;
>  
>   for (o = order; o <= 1; ++o) {

  ^ Knowing now that @order can only be 0 or 1 can this for loop (and
  goto) be dropped entirely?

  The code is already short and sweet so I don't feel strongly either
  way.

> @@ -712,7 +712,7 @@ static int mlx4_alloc_db_from_pgdir(struct mlx4_db_pgdir 
> *pgdir,
>   return 0;
>  }
>  
> -int mlx4_db_alloc(struct mlx4_dev *dev, struct mlx4_db *db, int order)
> +int mlx4_db_alloc(struct mlx4_dev *dev, struct mlx4_db *db, unsigned int 
> order)
>  {
>   struct mlx4_priv *priv = mlx4_priv(dev);
>   struct mlx4_db_pgdir *pgdir;
> diff --git a/include/linux/mlx4/device.h b/include/linux/mlx4/device.h
> index 27f42f713c89..86f0f2a25a3d 100644
> --- a/include/linux/mlx4/device.h
> +++ b/include/linux/mlx4/device.h
> @@ -1135,7 +1135,7 @@ int mlx4_write_mtt(struct mlx4_dev *dev, struct 
> mlx4_mtt *mtt,
>  int mlx4_buf_write_mtt(struct mlx4_dev *dev, struct mlx4_mtt *mtt,
>  struct mlx4_buf *buf);
>  
> -int mlx4_db_alloc(struct mlx4_dev *dev, struct mlx4_db *db, int order);
> +int mlx4_db_alloc(struct mlx4_dev *dev, struct mlx4_db *db, unsigned int 
> order);
>  void mlx4_db_free(struct mlx4_dev *dev, struct mlx4_db *db);
>  
>  int mlx4_alloc_hwq_res(struct mlx4_dev *dev, struct mlx4_hwq_resources 
> *wqres,
> -- 
> 2.34.1
> 

Justin



Re: [RESEND PATCH][next] fs: nfs: acl: Avoid -Wflex-array-member-not-at-end warning

2025-02-10 Thread Chuck Lever
On 2/3/25 10:03 PM, Gustavo A. R. Silva wrote:
> -Wflex-array-member-not-at-end was introduced in GCC-14, and we are
> getting ready to enable it, globally.
> 
> So, in order to avoid ending up with a flexible-array member in the
> middle of other structs, we use the `struct_group_tagged()` helper
> to create a new tagged `struct posix_acl_hdr`. This structure
> groups together all the members of the flexible `struct posix_acl`
> except the flexible array.
> 
> As a result, the array is effectively separated from the rest of the
> members without modifying the memory layout of the flexible structure.
> We then change the type of the middle struct member currently causing
> trouble from `struct posix_acl` to `struct posix_acl_hdr`.
> 
> We also want to ensure that when new members need to be added to the
> flexible structure, they are always included within the newly created
> tagged struct. For this, we use `static_assert()`. This ensures that the
> memory layout for both the flexible structure and the new tagged struct
> is the same after any changes.
> 
> This approach avoids having to implement `struct posix_acl_hdr` as a
> completely separate structure, thus preventing having to maintain two
> independent but basically identical structures, closing the door to
> potential bugs in the future.
> 
> We also use `container_of()` whenever we need to retrieve a pointer to
> the flexible structure, through which we can access the flexible-array
> member, if necessary.
> 
> So, with these changes, fix the following warning:
> 
> fs/nfs_common/nfsacl.c:45:26: warning: structure containing a flexible array 
> member is not at the end of another structure [-Wflex-array-member-not-at-end]
> 
> Signed-off-by: Gustavo A. R. Silva 
> ---
>  fs/nfs_common/nfsacl.c|  8 +---
>  include/linux/posix_acl.h | 11 ---
>  2 files changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/nfs_common/nfsacl.c b/fs/nfs_common/nfsacl.c
> index ea382b75b26c..e2eaac14fd8e 100644
> --- a/fs/nfs_common/nfsacl.c
> +++ b/fs/nfs_common/nfsacl.c
> @@ -42,7 +42,7 @@ struct nfsacl_encode_desc {
>  };
>  
>  struct nfsacl_simple_acl {
> - struct posix_acl acl;
> + struct posix_acl_hdr acl;
>   struct posix_acl_entry ace[4];
>  };
>  
> @@ -112,7 +112,8 @@ int nfsacl_encode(struct xdr_buf *buf, unsigned int base, 
> struct inode *inode,
>   xdr_encode_word(buf, base, entries))
>   return -EINVAL;
>   if (encode_entries && acl && acl->a_count == 3) {
> - struct posix_acl *acl2 = &aclbuf.acl;
> + struct posix_acl *acl2 =
> + container_of(&aclbuf.acl, struct posix_acl, hdr);
>  
>   /* Avoid the use of posix_acl_alloc().  nfsacl_encode() is
>* invoked in contexts where a memory allocation failure is
> @@ -177,7 +178,8 @@ bool nfs_stream_encode_acl(struct xdr_stream *xdr, struct 
> inode *inode,
>   return false;
>  
>   if (encode_entries && acl && acl->a_count == 3) {
> - struct posix_acl *acl2 = &aclbuf.acl;
> + struct posix_acl *acl2 =
> + container_of(&aclbuf.acl, struct posix_acl, hdr);
>  
>   /* Avoid the use of posix_acl_alloc().  nfsacl_encode() is
>* invoked in contexts where a memory allocation failure is
> diff --git a/include/linux/posix_acl.h b/include/linux/posix_acl.h
> index e2d47eb1a7f3..62d497763e25 100644
> --- a/include/linux/posix_acl.h
> +++ b/include/linux/posix_acl.h
> @@ -27,11 +27,16 @@ struct posix_acl_entry {
>  };
>  
>  struct posix_acl {
> - refcount_t  a_refcount;
> - unsigned inta_count;
> - struct rcu_head a_rcu;
> + /* New members MUST be added within the struct_group() macro below. */
> + struct_group_tagged(posix_acl_hdr, hdr,
> + refcount_t  a_refcount;
> + unsigned inta_count;
> + struct rcu_head a_rcu;
> + );
>   struct posix_acl_entry  a_entries[] __counted_by(a_count);
>  };
> +static_assert(offsetof(struct posix_acl, a_entries) == sizeof(struct 
> posix_acl_hdr),
> +   "struct member likely outside of struct_group_tagged()");
>  
>  #define FOREACH_ACL_ENTRY(pa, acl, pe) \
>   for(pa=(acl)->a_entries, pe=pa+(acl)->a_count; pa


-- 
Chuck Lever



Re: [PATCH v3 0/4] Allow default HARDENED_USERCOPY to be set at compile time

2025-02-10 Thread Kees Cook
On Thu, 23 Jan 2025 22:11:11 +, Mel Gorman wrote:
> Changelog since v2
> o Default on
> o Logic correction and simplification
> 
> Changelog since v1
> o Menu section rename
> o Make static branch usage similar to init_on_alloc
> o Change ordering of menu options
> 
> [...]

Applied to for-next/hardening, thanks!

[1/4] mm: security: Move hardened usercopy under 'Kernel hardening options'
  https://git.kernel.org/kees/c/8907c768bc27
[2/4] mm: security: Allow default HARDENED_USERCOPY to be set at compile time
  https://git.kernel.org/kees/c/caba7c35e832
[3/4] mm: security: Check early if HARDENED_USERCOPY is enabled
  https://git.kernel.org/kees/c/3d2220040476
[4/4] fortify: Move FORTIFY_SOURCE under 'Kernel hardening options'
  https://git.kernel.org/kees/c/b6518de35d6f

Take care,

-- 
Kees Cook




[PATCH] octeontx2-af: Fix uninitialized scalar variable

2025-02-10 Thread Ethan Carter Edwards
The variable *max_mtu* is uninitialized in the function
otx2_get_max_mtu. It is only assigned in the if-statement, leaving the
possibility of returning an uninitialized value.

1500 is the industry standard networking mtu and therefore should be the
default. If the function detects that the hardware custom sets the mtu,
then it will use it instead.

Addresses-Coverity-ID: 1636407 ("Uninitialized scalar variable")
Fixes: ab58a416c93f ("octeontx2-pf: cn10k: Get max mtu supported from admin 
function")
Signed-off-by: Ethan Carter Edwards 
---
 drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c 
b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
index 
2b49bfec78692cf1f63c793ec49511607cda7c3e..6c1b03690a9c24c5232ff9f07befb1cc553490f7
 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
@@ -1909,7 +1909,7 @@ u16 otx2_get_max_mtu(struct otx2_nic *pfvf)
 {
struct nix_hw_info *rsp;
struct msg_req *req;
-   u16 max_mtu;
+   u16 max_mtu = 1500;
int rc;
 
mutex_lock(&pfvf->mbox.lock);
@@ -1948,7 +1948,6 @@ u16 otx2_get_max_mtu(struct otx2_nic *pfvf)
if (rc) {
dev_warn(pfvf->dev,
 "Failed to get MTU from hardware setting default 
value(1500)\n");
-   max_mtu = 1500;
}
return max_mtu;
 }

---
base-commit: febbc555cf0fff895546ddb8ba2c9a523692fb55
change-id: 20250210-otx2_common-453132aa0a24

Best regards,
-- 
Ethan Carter Edwards 




Re: [PATCH v2 0/6] KUnit test moves / renames

2025-02-10 Thread Kees Cook
On Mon, Feb 10, 2025 at 05:11:24PM -0800, Andrew Morton wrote:
> On Mon, 10 Feb 2025 16:31:28 -0800 Kees Cook  wrote:
> 
> > This is rebased to v6.14-rc2. I'll carry it and folks can send me new
> > tests, etc, as needed to minimize future collisions.
> 
> Actually seems designed to maximize collisions.

Eek, that's not my intent!

> I'll drop ("lib/math: add Kunit test suite for gcd()")
> https://lore.kernel.org/all/0eaf6b69aeb2fe35092a633fed12537efe645303.1727332572.git.zhengqi.a...@bytedance.com/T/#u.

I will pick that up?

-- 
Kees Cook



[PATCH v2 1/6] lib: math: Move KUnit tests into tests/ subdir

2025-02-10 Thread Kees Cook
From: Luis Felipe Hernandez 

This patch is a follow-up task from a discussion stemming from point 3
in a recent patch introducing the int_pow kunit test [1] and
documentation regarding kunit test style and nomenclature [2].

Colocate all kunit test suites in lib/math/tests/ and
follow recommended naming convention for files _kunit.c
and kconfig entries CONFIG__KUNIT_TEST.

Link: 
https://lore.kernel.org/all/CABVgOS=-vh5TqHFCq_jo=ffq8v_nggr6jspnozag3e6+19y...@mail.gmail.com/
 [1]
Link: https://docs.kernel.org/dev-tools/kunit/style.html [2]

Signed-off-by: Luis Felipe Hernandez 
Acked-by: Nicolas Pitre 
Reviewed-by: David Gow 
Reviewed-by: Rae Moar 
Link: https://lore.kernel.org/r/20241202075545.3648096-2-david...@google.com
Signed-off-by: Kees Cook 
---
 lib/Kconfig.debug| 2 +-
 lib/math/Makefile| 5 ++---
 lib/math/tests/Makefile  | 3 ++-
 lib/math/{rational-test.c => tests/rational_kunit.c} | 0
 4 files changed, 5 insertions(+), 5 deletions(-)
 rename lib/math/{rational-test.c => tests/rational_kunit.c} (100%)

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 1af972a92d06..b18dbfc53ca4 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -3166,7 +3166,7 @@ config TEST_OBJPOOL
 
  If unsure, say N.
 
-config INT_POW_TEST
+config INT_POW_KUNIT_TEST
tristate "Integer exponentiation (int_pow) test" if !KUNIT_ALL_TESTS
depends on KUNIT
default KUNIT_ALL_TESTS
diff --git a/lib/math/Makefile b/lib/math/Makefile
index 853f023ae537..d1caba23baa0 100644
--- a/lib/math/Makefile
+++ b/lib/math/Makefile
@@ -5,8 +5,7 @@ obj-$(CONFIG_CORDIC)+= cordic.o
 obj-$(CONFIG_PRIME_NUMBERS)+= prime_numbers.o
 obj-$(CONFIG_RATIONAL) += rational.o
 
-obj-$(CONFIG_INT_POW_TEST)  += tests/int_pow_kunit.o
 obj-$(CONFIG_TEST_DIV64)   += test_div64.o
 obj-$(CONFIG_TEST_MULDIV64)+= test_mul_u64_u64_div_u64.o
-obj-$(CONFIG_RATIONAL_KUNIT_TEST) += rational-test.o
-obj-$(CONFIG_INT_SQRT_KUNIT_TEST) += tests/int_sqrt_kunit.o
\ No newline at end of file
+
+obj-y += tests/
diff --git a/lib/math/tests/Makefile b/lib/math/tests/Makefile
index e1a79f093b2d..66b10a7f5a84 100644
--- a/lib/math/tests/Makefile
+++ b/lib/math/tests/Makefile
@@ -1,4 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0-only
 
-obj-$(CONFIG_INT_POW_TEST) += int_pow_kunit.o
+obj-$(CONFIG_INT_POW_KUNIT_TEST)  += int_pow_kunit.o
+obj-$(CONFIG_RATIONAL_KUNIT_TEST) += rational_kunit.o
 obj-$(CONFIG_INT_SQRT_KUNIT_TEST) += int_sqrt_kunit.o
diff --git a/lib/math/rational-test.c b/lib/math/tests/rational_kunit.c
similarity index 100%
rename from lib/math/rational-test.c
rename to lib/math/tests/rational_kunit.c
-- 
2.34.1




[PATCH v2 3/6] lib: Move KUnit tests into tests/ subdirectory

2025-02-10 Thread Kees Cook
Following from the recent KUnit file naming discussion[1], move all
KUnit tests in lib/ into lib/tests/.

Link: https://lore.kernel.org/lkml/20240720165441.it.320-k...@kernel.org/ [1]
Acked-by: Steven Rostedt (Google) 
Acked-by: Jakub Kicinski 
Acked-by: Masami Hiramatsu (Google) 
Reviewed-by: David Gow 
Acked-by: Vlastimil Babka 
Reviewed-by: Rae Moar 
Link: https://lore.kernel.org/r/20241202075545.3648096-4-david...@google.com
Signed-off-by: Kees Cook 
---
 MAINTAINERS| 19 ++--
 lib/Makefile   | 39 +
 lib/tests/Makefile | 40 ++
 lib/{ => tests}/bitfield_kunit.c   |  0
 lib/{ => tests}/checksum_kunit.c   |  0
 lib/{ => tests}/cmdline_kunit.c|  0
 lib/{ => tests}/cpumask_kunit.c|  0
 lib/{ => tests}/crc_kunit.c|  0
 lib/{ => tests}/fortify_kunit.c|  0
 lib/{ => tests}/hashtable_test.c   |  0
 lib/{ => tests}/is_signed_type_kunit.c |  0
 lib/{ => tests}/kunit_iov_iter.c   |  0
 lib/{ => tests}/list-test.c|  0
 lib/{ => tests}/memcpy_kunit.c |  0
 lib/{ => tests}/overflow_kunit.c   |  0
 lib/{ => tests}/siphash_kunit.c|  0
 lib/{ => tests}/slub_kunit.c   |  0
 lib/{ => tests}/stackinit_kunit.c  |  0
 lib/{ => tests}/string_helpers_kunit.c |  0
 lib/{ => tests}/string_kunit.c |  0
 lib/{ => tests}/test_bits.c|  0
 lib/{ => tests}/test_fprobe.c  |  0
 lib/{ => tests}/test_hash.c|  0
 lib/{ => tests}/test_kprobes.c |  0
 lib/{ => tests}/test_linear_ranges.c   |  0
 lib/{ => tests}/test_list_sort.c   |  0
 lib/{ => tests}/test_sort.c|  0
 lib/{ => tests}/usercopy_kunit.c   |  0
 lib/{ => tests}/util_macros_kunit.c|  0
 29 files changed, 51 insertions(+), 47 deletions(-)
 rename lib/{ => tests}/bitfield_kunit.c (100%)
 rename lib/{ => tests}/checksum_kunit.c (100%)
 rename lib/{ => tests}/cmdline_kunit.c (100%)
 rename lib/{ => tests}/cpumask_kunit.c (100%)
 rename lib/{ => tests}/crc_kunit.c (100%)
 rename lib/{ => tests}/fortify_kunit.c (100%)
 rename lib/{ => tests}/hashtable_test.c (100%)
 rename lib/{ => tests}/is_signed_type_kunit.c (100%)
 rename lib/{ => tests}/kunit_iov_iter.c (100%)
 rename lib/{ => tests}/list-test.c (100%)
 rename lib/{ => tests}/memcpy_kunit.c (100%)
 rename lib/{ => tests}/overflow_kunit.c (100%)
 rename lib/{ => tests}/siphash_kunit.c (100%)
 rename lib/{ => tests}/slub_kunit.c (100%)
 rename lib/{ => tests}/stackinit_kunit.c (100%)
 rename lib/{ => tests}/string_helpers_kunit.c (100%)
 rename lib/{ => tests}/string_kunit.c (100%)
 rename lib/{ => tests}/test_bits.c (100%)
 rename lib/{ => tests}/test_fprobe.c (100%)
 rename lib/{ => tests}/test_hash.c (100%)
 rename lib/{ => tests}/test_kprobes.c (100%)
 rename lib/{ => tests}/test_linear_ranges.c (100%)
 rename lib/{ => tests}/test_list_sort.c (100%)
 rename lib/{ => tests}/test_sort.c (100%)
 rename lib/{ => tests}/usercopy_kunit.c (100%)
 rename lib/{ => tests}/util_macros_kunit.c (100%)

diff --git a/MAINTAINERS b/MAINTAINERS
index 25c86f47353d..78be36fb1a74 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4017,10 +4017,10 @@ F:  include/vdso/bits.h
 F: lib/bitmap-str.c
 F: lib/bitmap.c
 F: lib/cpumask.c
-F: lib/cpumask_kunit.c
 F: lib/find_bit.c
 F: lib/find_bit_benchmark.c
 F: lib/test_bitmap.c
+F: lib/tests/cpumask_kunit.c
 F: tools/include/linux/bitfield.h
 F: tools/include/linux/bitmap.h
 F: tools/include/linux/bits.h
@@ -9065,9 +9065,10 @@ L:   linux-hardening@vger.kernel.org
 S: Supported
 T: git git://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git 
for-next/hardening
 F: include/linux/fortify-string.h
-F: lib/fortify_kunit.c
-F: lib/memcpy_kunit.c
 F: lib/test_fortify/*
+F: lib/tests/fortify_kunit.c
+F: lib/tests/memcpy_kunit.c
+F: scripts/test_fortify.sh
 K: \bunsafe_memcpy\b
 K: \b__NO_FORTIFY\b
 
@@ -9755,9 +9756,9 @@ F:include/linux/string.h
 F: include/linux/string_choices.h
 F: include/linux/string_helpers.h
 F: lib/string.c
-F: lib/string_kunit.c
 F: lib/string_helpers.c
-F: lib/string_helpers_kunit.c
+F: lib/tests/string_helpers_kunit.c
+F: lib/tests/string_kunit.c
 F: scripts/coccinelle/api/string_choices.cocci
 
 GENERIC UIO DRIVER FOR PCI DEVICES
@@ -12985,7 +12986,7 @@ F:  Documentation/trace/kprobes.rst
 F: include/asm-generic/kprobes.h
 F: include/linux/kprobes.h
 F: kernel/kprobes.c
-F: lib/test_kprobes.c
+F: lib/tests/test_kprobes.c
 F: samples/kprobes
 
 KS0108 LCD CONTROLLER DRIVER
@@ -13315,7 +13316,7 @@ M:  Mark Brown 
 R: Matti Vaittinen 
 F: include/linux/linear_range.h
 F: lib/linear_ranges.c
-F: lib/test_linear_ranges.c
+F: lib/tests/test_linear_ranges.c
 
 LINUX FOR POWER MACINTOSH
 L: linuxppc-...@lists.o

[PATCH v2 0/6] KUnit test moves / renames

2025-02-10 Thread Kees Cook
Hi,

This is rebased to v6.14-rc2. I'll carry it and folks can send me new
tests, etc, as needed to minimize future collisions.

 v1: https://lore.kernel.org/lkml/20241011072509.3068328-2-david...@google.com/

Thanks!

-Kees

Bruno Sobreira França (1):
  lib/math: Add int_log test suite

Diego Vieira (1):
  lib/tests/kfifo_kunit.c: add tests for the kfifo structure

Gabriela Bittencourt (2):
  unicode: kunit: refactor selftest to kunit tests
  unicode: kunit: change tests filename and path

Kees Cook (1):
  lib: Move KUnit tests into tests/ subdirectory

Luis Felipe Hernandez (1):
  lib: math: Move KUnit tests into tests/ subdir

 MAINTAINERS   |  19 +-
 fs/unicode/Kconfig|   5 +-
 fs/unicode/Makefile   |   2 +-
 fs/unicode/tests/.kunitconfig |   3 +
 .../{utf8-selftest.c => tests/utf8_kunit.c}   | 149 ++--
 fs/unicode/utf8-norm.c|   2 +-
 lib/Kconfig.debug |  27 ++-
 lib/Makefile  |  39 +--
 lib/math/Makefile |   5 +-
 lib/math/tests/Makefile   |   4 +-
 lib/math/tests/int_log_kunit.c|  74 ++
 .../rational_kunit.c} |   0
 lib/tests/Makefile|  41 
 lib/{ => tests}/bitfield_kunit.c  |   0
 lib/{ => tests}/checksum_kunit.c  |   0
 lib/{ => tests}/cmdline_kunit.c   |   0
 lib/{ => tests}/cpumask_kunit.c   |   0
 lib/{ => tests}/crc_kunit.c   |   0
 lib/{ => tests}/fortify_kunit.c   |   0
 lib/{ => tests}/hashtable_test.c  |   0
 lib/{ => tests}/is_signed_type_kunit.c|   0
 lib/tests/kfifo_kunit.c   | 224 ++
 lib/{ => tests}/kunit_iov_iter.c  |   0
 lib/{ => tests}/list-test.c   |   0
 lib/{ => tests}/memcpy_kunit.c|   0
 lib/{ => tests}/overflow_kunit.c  |   0
 lib/{ => tests}/siphash_kunit.c   |   0
 lib/{ => tests}/slub_kunit.c  |   0
 lib/{ => tests}/stackinit_kunit.c |   0
 lib/{ => tests}/string_helpers_kunit.c|   0
 lib/{ => tests}/string_kunit.c|   0
 lib/{ => tests}/test_bits.c   |   0
 lib/{ => tests}/test_fprobe.c |   0
 lib/{ => tests}/test_hash.c   |   0
 lib/{ => tests}/test_kprobes.c|   0
 lib/{ => tests}/test_linear_ranges.c  |   0
 lib/{ => tests}/test_list_sort.c  |   0
 lib/{ => tests}/test_sort.c   |   0
 lib/{ => tests}/usercopy_kunit.c  |   0
 lib/{ => tests}/util_macros_kunit.c   |   0
 40 files changed, 458 insertions(+), 136 deletions(-)
 create mode 100644 fs/unicode/tests/.kunitconfig
 rename fs/unicode/{utf8-selftest.c => tests/utf8_kunit.c} (64%)
 create mode 100644 lib/math/tests/int_log_kunit.c
 rename lib/math/{rational-test.c => tests/rational_kunit.c} (100%)
 rename lib/{ => tests}/bitfield_kunit.c (100%)
 rename lib/{ => tests}/checksum_kunit.c (100%)
 rename lib/{ => tests}/cmdline_kunit.c (100%)
 rename lib/{ => tests}/cpumask_kunit.c (100%)
 rename lib/{ => tests}/crc_kunit.c (100%)
 rename lib/{ => tests}/fortify_kunit.c (100%)
 rename lib/{ => tests}/hashtable_test.c (100%)
 rename lib/{ => tests}/is_signed_type_kunit.c (100%)
 create mode 100644 lib/tests/kfifo_kunit.c
 rename lib/{ => tests}/kunit_iov_iter.c (100%)
 rename lib/{ => tests}/list-test.c (100%)
 rename lib/{ => tests}/memcpy_kunit.c (100%)
 rename lib/{ => tests}/overflow_kunit.c (100%)
 rename lib/{ => tests}/siphash_kunit.c (100%)
 rename lib/{ => tests}/slub_kunit.c (100%)
 rename lib/{ => tests}/stackinit_kunit.c (100%)
 rename lib/{ => tests}/string_helpers_kunit.c (100%)
 rename lib/{ => tests}/string_kunit.c (100%)
 rename lib/{ => tests}/test_bits.c (100%)
 rename lib/{ => tests}/test_fprobe.c (100%)
 rename lib/{ => tests}/test_hash.c (100%)
 rename lib/{ => tests}/test_kprobes.c (100%)
 rename lib/{ => tests}/test_linear_ranges.c (100%)
 rename lib/{ => tests}/test_list_sort.c (100%)
 rename lib/{ => tests}/test_sort.c (100%)
 rename lib/{ => tests}/usercopy_kunit.c (100%)
 rename lib/{ => tests}/util_macros_kunit.c (100%)

-- 
2.34.1




[PATCH v2 4/6] lib/tests/kfifo_kunit.c: add tests for the kfifo structure

2025-02-10 Thread Kees Cook
From: Diego Vieira 

Add KUnit tests for the kfifo data structure.
They test the vast majority of macros defined in the kfifo
header (include/linux/kfifo.h).

These are inspired by the existing tests for the doubly
linked list in lib/tests/list-test.c (previously at lib/list-test.c) [1].

Note that this patch depends on the patch that moves the KUnit tests on
lib/ into lib/tests/ [2].

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/lib/list-test.c?h=v6.11-rc6
[2] https://lore.kernel.org/all/20240720181025.work.002-k...@kernel.org/

Signed-off-by: Diego Vieira 
Reviewed-by: David Gow 
Reviewed-by: Rae Moar 
Link: https://lore.kernel.org/r/20241202075545.3648096-5-david...@google.com
Signed-off-by: Kees Cook 
---
 lib/Kconfig.debug   |  14 +++
 lib/tests/Makefile  |   1 +
 lib/tests/kfifo_kunit.c | 224 
 3 files changed, 239 insertions(+)
 create mode 100644 lib/tests/kfifo_kunit.c

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index f30929ed1a84..86ddf568cbca 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -2691,6 +2691,20 @@ config SYSCTL_KUNIT_TEST
 
  If unsure, say N.
 
+config KFIFO_KUNIT_TEST
+   tristate "KUnit Test for the generic kernel FIFO implementation" if 
!KUNIT_ALL_TESTS
+   depends on KUNIT
+   default KUNIT_ALL_TESTS
+   help
+ This builds the generic FIFO implementation KUnit test suite.
+ It tests that the API and basic functionality of the kfifo type
+ and associated macros.
+
+ For more information on KUnit and unit tests in general please refer
+ to the KUnit documentation in Documentation/dev-tools/kunit/.
+
+ If unsure, say N.
+
 config LIST_KUNIT_TEST
tristate "KUnit Test for Kernel Linked-list structures" if 
!KUNIT_ALL_TESTS
depends on KUNIT
diff --git a/lib/tests/Makefile b/lib/tests/Makefile
index c7b5170359c1..8696d778d92f 100644
--- a/lib/tests/Makefile
+++ b/lib/tests/Makefile
@@ -23,6 +23,7 @@ obj-$(CONFIG_TEST_IOV_ITER) += kunit_iov_iter.o
 obj-$(CONFIG_IS_SIGNED_TYPE_KUNIT_TEST) += is_signed_type_kunit.o
 obj-$(CONFIG_KPROBES_SANITY_TEST) += test_kprobes.o
 obj-$(CONFIG_LIST_KUNIT_TEST) += list-test.o
+obj-$(CONFIG_KFIFO_KUNIT_TEST) += kfifo_kunit.o
 obj-$(CONFIG_TEST_LIST_SORT) += test_list_sort.o
 obj-$(CONFIG_LINEAR_RANGES_TEST) += test_linear_ranges.o
 obj-$(CONFIG_MEMCPY_KUNIT_TEST) += memcpy_kunit.o
diff --git a/lib/tests/kfifo_kunit.c b/lib/tests/kfifo_kunit.c
new file mode 100644
index ..a85eedc3195a
--- /dev/null
+++ b/lib/tests/kfifo_kunit.c
@@ -0,0 +1,224 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * KUnit test for the generic kernel FIFO implementation.
+ *
+ * Copyright (C) 2024 Diego Vieira 
+ */
+#include 
+
+#include 
+
+#define KFIFO_SIZE 32
+#define N_ELEMENTS 5
+
+static void kfifo_test_reset_should_clear_the_fifo(struct kunit *test)
+{
+   DEFINE_KFIFO(my_fifo, u8, KFIFO_SIZE);
+
+   kfifo_put(&my_fifo, 1);
+   kfifo_put(&my_fifo, 2);
+   kfifo_put(&my_fifo, 3);
+   KUNIT_EXPECT_EQ(test, kfifo_len(&my_fifo), 3);
+
+   kfifo_reset(&my_fifo);
+
+   KUNIT_EXPECT_EQ(test, kfifo_len(&my_fifo), 0);
+   KUNIT_EXPECT_TRUE(test, kfifo_is_empty(&my_fifo));
+}
+
+static void kfifo_test_define_should_define_an_empty_fifo(struct kunit *test)
+{
+   DEFINE_KFIFO(my_fifo, u8, KFIFO_SIZE);
+
+   KUNIT_EXPECT_TRUE(test, kfifo_initialized(&my_fifo));
+   KUNIT_EXPECT_TRUE(test, kfifo_is_empty(&my_fifo));
+   KUNIT_EXPECT_EQ(test, kfifo_len(&my_fifo), 0);
+}
+
+static void kfifo_test_len_should_ret_n_of_stored_elements(struct kunit *test)
+{
+   u8 buffer1[N_ELEMENTS];
+
+   for (int i = 0; i < N_ELEMENTS; i++)
+   buffer1[i] = i + 1;
+
+   DEFINE_KFIFO(my_fifo, u8, KFIFO_SIZE);
+
+   KUNIT_EXPECT_EQ(test, kfifo_len(&my_fifo), 0);
+
+   kfifo_in(&my_fifo, buffer1, N_ELEMENTS);
+   KUNIT_EXPECT_EQ(test, kfifo_len(&my_fifo), N_ELEMENTS);
+
+   kfifo_in(&my_fifo, buffer1, N_ELEMENTS);
+   KUNIT_EXPECT_EQ(test, kfifo_len(&my_fifo), N_ELEMENTS * 2);
+
+   kfifo_reset(&my_fifo);
+   KUNIT_EXPECT_EQ(test, kfifo_len(&my_fifo), 0);
+}
+
+static void kfifo_test_put_should_insert_and_get_should_pop(struct kunit *test)
+{
+   u8 out_data = 0;
+   int processed_elements;
+   u8 elements[] = { 3, 5, 11 };
+
+   DEFINE_KFIFO(my_fifo, u8, KFIFO_SIZE);
+
+   // If the fifo is empty, get returns 0
+   processed_elements = kfifo_get(&my_fifo, &out_data);
+   KUNIT_EXPECT_EQ(test, processed_elements, 0);
+   KUNIT_EXPECT_EQ(test, out_data, 0);
+
+   for (int i = 0; i < 3; i++)
+   kfifo_put(&my_fifo, elements[i]);
+
+   for (int i = 0; i < 3; i++) {
+   processed_elements = kfifo_get(&my_fifo, &out_data);
+   KUNIT_EXPECT_EQ(test, processed_elements, 1);
+   KUNIT_EXPECT_EQ(test, out_data, elements[i]);
+   }
+

[PATCH v2 5/6] unicode: kunit: refactor selftest to kunit tests

2025-02-10 Thread Kees Cook
From: Gabriela Bittencourt 

Refactoring 'test' functions into kunit tests, to test utf-8 support in
unicode subsystem.

This allows the utf8 tests to be run alongside the KUnit test suite
using kunit-tool, quickly compiling and running all desired tests as
part of the KUnit test suite, instead of compiling the selftest module
and loading it.

The refactoring kept the original testing logic intact, while adopting a
testing pattern across different kernel modules and leveraging KUnit's
benefits.

Co-developed-by: Pedro Orlando 
Signed-off-by: Pedro Orlando 
Co-developed-by: Danilo Pereira 
Signed-off-by: Danilo Pereira 
Signed-off-by: Gabriela Bittencourt 
Reviewed-by: David Gow 
Acked-by: Gabriel Krisman Bertazi 
Reviewed-by: Rae Moar 
Link: https://lore.kernel.org/r/20241202075545.3648096-6-david...@google.com
Signed-off-by: Kees Cook 
---
 fs/unicode/.kunitconfig|   3 +
 fs/unicode/Kconfig |   5 +-
 fs/unicode/Makefile|   2 +-
 fs/unicode/utf8-norm.c |   2 +-
 fs/unicode/utf8-selftest.c | 149 +
 5 files changed, 77 insertions(+), 84 deletions(-)
 create mode 100644 fs/unicode/.kunitconfig

diff --git a/fs/unicode/.kunitconfig b/fs/unicode/.kunitconfig
new file mode 100644
index ..62dd5c171f9c
--- /dev/null
+++ b/fs/unicode/.kunitconfig
@@ -0,0 +1,3 @@
+CONFIG_KUNIT=y
+CONFIG_UNICODE=y
+CONFIG_UNICODE_NORMALIZATION_KUNIT_TEST=y
diff --git a/fs/unicode/Kconfig b/fs/unicode/Kconfig
index da786a687fdc..4ad2c36550f1 100644
--- a/fs/unicode/Kconfig
+++ b/fs/unicode/Kconfig
@@ -10,6 +10,7 @@ config UNICODE
  be a separate loadable module that gets requested only when a file
  system actually use it.
 
-config UNICODE_NORMALIZATION_SELFTEST
+config UNICODE_NORMALIZATION_KUNIT_TEST
tristate "Test UTF-8 normalization support"
-   depends on UNICODE
+   depends on UNICODE && KUNIT
+   default KUNIT_ALL_TESTS
diff --git a/fs/unicode/Makefile b/fs/unicode/Makefile
index e309afe2b2bb..37bbcbc628a1 100644
--- a/fs/unicode/Makefile
+++ b/fs/unicode/Makefile
@@ -4,7 +4,7 @@ ifneq ($(CONFIG_UNICODE),)
 obj-y  += unicode.o
 endif
 obj-$(CONFIG_UNICODE)  += utf8data.o
-obj-$(CONFIG_UNICODE_NORMALIZATION_SELFTEST) += utf8-selftest.o
+obj-$(CONFIG_UNICODE_NORMALIZATION_KUNIT_TEST) += utf8-selftest.o
 
 unicode-y := utf8-norm.o utf8-core.o
 
diff --git a/fs/unicode/utf8-norm.c b/fs/unicode/utf8-norm.c
index 768f8ab448b8..7b998c99c88d 100644
--- a/fs/unicode/utf8-norm.c
+++ b/fs/unicode/utf8-norm.c
@@ -586,7 +586,7 @@ int utf8byte(struct utf8cursor *u8c)
}
 }
 
-#ifdef CONFIG_UNICODE_NORMALIZATION_SELFTEST_MODULE
+#if IS_MODULE(CONFIG_UNICODE_NORMALIZATION_KUNIT_TEST)
 EXPORT_SYMBOL_GPL(utf8version_is_supported);
 EXPORT_SYMBOL_GPL(utf8nlen);
 EXPORT_SYMBOL_GPL(utf8ncursor);
diff --git a/fs/unicode/utf8-selftest.c b/fs/unicode/utf8-selftest.c
index 5ddaf27b21a6..9476ab012baa 100644
--- a/fs/unicode/utf8-selftest.c
+++ b/fs/unicode/utf8-selftest.c
@@ -1,35 +1,15 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /*
- * Kernel module for testing utf-8 support.
+ * KUnit tests for utf-8 support.
  *
  * Copyright 2017 Collabora Ltd.
  */
 
-#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
-
-#include 
-#include 
 #include 
-#include 
+#include 
 
 #include "utf8n.h"
 
-static unsigned int failed_tests;
-static unsigned int total_tests;
-
-#define _test(cond, func, line, fmt, ...) do { \
-   total_tests++;  \
-   if (!cond) {\
-   failed_tests++; \
-   pr_err("test %s:%d Failed: %s%s",   \
-  func, line, #cond, (fmt?":":"."));   \
-   if (fmt)\
-   pr_err(fmt, ##__VA_ARGS__); \
-   }   \
-   } while (0)
-#define test_f(cond, fmt, ...) _test(cond, __func__, __LINE__, fmt, 
##__VA_ARGS__)
-#define test(cond) _test(cond, __func__, __LINE__, "")
-
 static const struct {
/* UTF-8 strings in this vector _must_ be NULL-terminated. */
unsigned char str[10];
@@ -167,69 +147,74 @@ static int utf8cursor(struct utf8cursor *u8c, const 
struct unicode_map *um,
return utf8ncursor(u8c, um, n, s, (unsigned int)-1);
 }
 
-static void check_utf8_nfdi(struct unicode_map *um)
+static void check_utf8_nfdi(struct kunit *test)
 {
int i;
struct utf8cursor u8c;
+   struct unicode_map *um = test->priv;
 
for (i = 0; i < ARRAY_SIZE(nfdi_test_data); i++) {
int len = strlen(nfdi_test_data[i].str);
int nlen = strlen(nfdi_test_data[i].dec);
int j = 0;
unsigned char c;
+   int ret;
+
+   KUNIT_EXPEC

[PATCH v2 6/6] unicode: kunit: change tests filename and path

2025-02-10 Thread Kees Cook
From: Gabriela Bittencourt 

Change utf8 kunit test filename and path to follow the style
convention on Documentation/dev-tools/kunit/style.rst

Co-developed-by: Pedro Orlando 
Signed-off-by: Pedro Orlando 
Co-developed-by: Danilo Pereira 
Signed-off-by: Danilo Pereira 
Signed-off-by: Gabriela Bittencourt 
Reviewed-by: David Gow 
Acked-by: Gabriel Krisman Bertazi 
Reviewed-by: Shuah Khan 
Reviewed-by: Rae Moar 
Link: https://lore.kernel.org/r/20241202075545.3648096-7-david...@google.com
Signed-off-by: Kees Cook 
---
 fs/unicode/Makefile| 2 +-
 fs/unicode/{ => tests}/.kunitconfig| 0
 fs/unicode/{utf8-selftest.c => tests/utf8_kunit.c} | 0
 3 files changed, 1 insertion(+), 1 deletion(-)
 rename fs/unicode/{ => tests}/.kunitconfig (100%)
 rename fs/unicode/{utf8-selftest.c => tests/utf8_kunit.c} (100%)

diff --git a/fs/unicode/Makefile b/fs/unicode/Makefile
index 37bbcbc628a1..d95be7fb9f6b 100644
--- a/fs/unicode/Makefile
+++ b/fs/unicode/Makefile
@@ -4,7 +4,7 @@ ifneq ($(CONFIG_UNICODE),)
 obj-y  += unicode.o
 endif
 obj-$(CONFIG_UNICODE)  += utf8data.o
-obj-$(CONFIG_UNICODE_NORMALIZATION_KUNIT_TEST) += utf8-selftest.o
+obj-$(CONFIG_UNICODE_NORMALIZATION_KUNIT_TEST) += tests/utf8_kunit.o
 
 unicode-y := utf8-norm.o utf8-core.o
 
diff --git a/fs/unicode/.kunitconfig b/fs/unicode/tests/.kunitconfig
similarity index 100%
rename from fs/unicode/.kunitconfig
rename to fs/unicode/tests/.kunitconfig
diff --git a/fs/unicode/utf8-selftest.c b/fs/unicode/tests/utf8_kunit.c
similarity index 100%
rename from fs/unicode/utf8-selftest.c
rename to fs/unicode/tests/utf8_kunit.c
-- 
2.34.1




[PATCH v2 2/6] lib/math: Add int_log test suite

2025-02-10 Thread Kees Cook
From: Bruno Sobreira França 

This commit introduces KUnit tests for the intlog2 and intlog10
functions, which compute logarithms in base 2 and base 10, respectively.
The tests cover a range of inputs to ensure the correctness of these
functions across common and edge cases.

Signed-off-by: Bruno Sobreira França 
Reviewed-by: David Gow 
Reviewed-by: Rae Moar 
Link: https://lore.kernel.org/r/20241202075545.3648096-3-david...@google.com
Signed-off-by: Kees Cook 
---
 lib/Kconfig.debug  | 11 +
 lib/math/tests/Makefile|  1 +
 lib/math/tests/int_log_kunit.c | 74 ++
 3 files changed, 86 insertions(+)
 create mode 100644 lib/math/tests/int_log_kunit.c

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index b18dbfc53ca4..f30929ed1a84 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -3197,6 +3197,17 @@ config INT_SQRT_KUNIT_TEST
 
  If unsure, say N
 
+config INT_LOG_KUNIT_TEST
+tristate "Integer log (int_log) test" if !KUNIT_ALL_TESTS
+depends on KUNIT
+default KUNIT_ALL_TESTS
+help
+  This option enables the KUnit test suite for the int_log library, 
which
+  provides two functions to compute the integer logarithm in base 2 and
+  base 10, called respectively as intlog2 and intlog10.
+
+  If unsure, say N
+
 endif # RUNTIME_TESTING_MENU
 
 config ARCH_USE_MEMTEST
diff --git a/lib/math/tests/Makefile b/lib/math/tests/Makefile
index 66b10a7f5a84..f139b73c65cb 100644
--- a/lib/math/tests/Makefile
+++ b/lib/math/tests/Makefile
@@ -1,5 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0-only
 
 obj-$(CONFIG_INT_POW_KUNIT_TEST)  += int_pow_kunit.o
+obj-$(CONFIG_INT_LOG_KUNIT_TEST)  += int_log_kunit.o
 obj-$(CONFIG_RATIONAL_KUNIT_TEST) += rational_kunit.o
 obj-$(CONFIG_INT_SQRT_KUNIT_TEST) += int_sqrt_kunit.o
diff --git a/lib/math/tests/int_log_kunit.c b/lib/math/tests/int_log_kunit.c
new file mode 100644
index ..14e854146cb4
--- /dev/null
+++ b/lib/math/tests/int_log_kunit.c
@@ -0,0 +1,74 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include 
+#include 
+
+struct test_case_params {
+   u32 value;
+   unsigned int expected_result;
+   const char *name;
+};
+
+
+/* The expected result takes into account the log error */
+static const struct test_case_params intlog2_params[] = {
+   {0, 0, "Log base 2 of 0"},
+   {1, 0, "Log base 2 of 1"},
+   {2, 16777216, "Log base 2 of 2"},
+   {3, 26591232, "Log base 2 of 3"},
+   {4, 33554432, "Log base 2 of 4"},
+   {8, 50331648, "Log base 2 of 8"},
+   {16, 67108864, "Log base 2 of 16"},
+   {32, 83886080, "Log base 2 of 32"},
+   {U32_MAX, 536870911, "Log base 2 of MAX"},
+};
+
+static const struct test_case_params intlog10_params[] = {
+   {0, 0, "Log base 10 of 0"},
+   {1, 0, "Log base 10 of 1"},
+   {6, 13055203, "Log base 10 of 6"},
+   {10, 16777225, "Log base 10 of 10"},
+   {100, 33554450, "Log base 10 of 100"},
+   {1000, 50331675, "Log base 10 of 1000"},
+   {1, 67108862, "Log base 10 of 1"},
+   {U32_MAX, 161614247, "Log base 10 of MAX"}
+};
+
+static void get_desc(const struct test_case_params *tc, char *desc)
+{
+   strscpy(desc, tc->name, KUNIT_PARAM_DESC_SIZE);
+}
+
+
+KUNIT_ARRAY_PARAM(intlog2, intlog2_params, get_desc);
+
+static void intlog2_test(struct kunit *test)
+{
+   const struct test_case_params *tc = (const struct test_case_params 
*)test->param_value;
+
+   KUNIT_EXPECT_EQ(test, tc->expected_result, intlog2(tc->value));
+}
+
+KUNIT_ARRAY_PARAM(intlog10, intlog10_params, get_desc);
+
+static void intlog10_test(struct kunit *test)
+{
+   const struct test_case_params *tc = (const struct test_case_params 
*)test->param_value;
+
+   KUNIT_EXPECT_EQ(test, tc->expected_result, intlog10(tc->value));
+}
+
+static struct kunit_case math_int_log_test_cases[] = {
+   KUNIT_CASE_PARAM(intlog2_test, intlog2_gen_params),
+   KUNIT_CASE_PARAM(intlog10_test, intlog10_gen_params),
+   {}
+};
+
+static struct kunit_suite int_log_test_suite = {
+   .name = "math-int_log",
+   .test_cases =  math_int_log_test_cases,
+};
+
+kunit_test_suites(&int_log_test_suite);
+
+MODULE_DESCRIPTION("math.int_log KUnit test suite");
+MODULE_LICENSE("GPL");
-- 
2.34.1




Re: [PATCH v4 29/30] x86/mm, mm/vmalloc: Defer flush_tlb_kernel_range() targeting NOHZ_FULL CPUs

2025-02-10 Thread Jann Horn
On Mon, Feb 10, 2025 at 7:36 PM Valentin Schneider  wrote:
> What if isolated CPUs unconditionally did a TLBi as late as possible in
> the stack right before returning to userspace? This would mean that upon
> re-entering the kernel, an isolated CPU's TLB wouldn't contain any kernel
> range translation - with the exception of whatever lies between the
> last-minute flush and the actual userspace entry, which should be feasible
> to vet? Then AFAICT there wouldn't be any work/flush to defer, the IPI
> could be entirely silenced if it targets an isolated CPU.

Two issues with that:

1. I think the "Common not Private" feature Will Deacon referred to is
incompatible with this idea:

says "When the CnP bit is set, the software promises to use the ASIDs
and VMIDs in the same way on all processors, which allows the TLB
entries that are created by one processor to be used by another"

2. It's wrong to assume that TLB entries are only populated for
addresses you access - thanks to speculative execution, you have to
assume that the CPU might be populating random TLB entries all over
the place.



RE: [PATCH] wifi: ath12k: Fix uninitialized variable and remove goto

2025-02-10 Thread Ping-Ke Shih
> > >
> > > Addresses-Coverity-ID: 1642337 ("Uninitialized scalar variable")
> >
> > Is that an official kernel tag? IMO the proper tag would be
> So, it isn't "official" as far as I can tell, but it is widely used in
> other commits, especially by Gustavo Silva.
> 
> Also:
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=778e2478d19574508861bcb
> 1806d6e34d095587c
> 
> Coverity-IDs: is another option I have found. I have seen Closes: a few
> times as well. I'm not really sure what the best option is, honestly.

In my patch, I used and treated Addresses-Coverity-ID as a unofficial tag,
so additional empty line is added.

Days ago I have received Coverity issues sent to mailing list, so I used
Closes tag at that time. But recently I have not seen that kind of mails. 
Instead, I visit Coverity web site to check issues and use
Addresses-Coverity-ID tag, since Coverity link is not visible to everyone.
That is just my thought. 





Re: [PATCH v3][next] tty: tty_buffer: Avoid hundreds of -Wflex-array-member-not-at-end warnings

2025-02-10 Thread Gustavo A. R. Silva








@@ -576,11 +579,14 @@ int tty_insert_flip_string_and_push_buffer(struct 
tty_port *port,
  void tty_buffer_init(struct tty_port *port)
  {
  struct tty_bufhead *buf = &port->buf;
+    struct tty_buffer *buf_sentinel;
+
+    buf_sentinel = container_of(&buf->sentinel, struct tty_buffer, hdr);


Bah, so this is ugly and even dangerous if someone adds a member to tty_buffer 
outside _hdr.

We should link headers in the list, it appears.


What should that patch look like?

--
Gustavo



Re: [PATCH v2 0/6] KUnit test moves / renames

2025-02-10 Thread David Gow
On Tue, 11 Feb 2025 at 08:31, Kees Cook  wrote:
>
> Hi,
>
> This is rebased to v6.14-rc2. I'll carry it and folks can send me new
> tests, etc, as needed to minimize future collisions.
>
>  v1: 
> https://lore.kernel.org/lkml/20241011072509.3068328-2-david...@google.com/
>
> Thanks!
>
> -Kees

Thanks a lot, Kees -- this has been languishing for too long!

I'm afraid that there might still end up being one or two conflicts,
particularly around the maths tests and crc tests, both of which are
seeing some development. I've CCed the folks writing patches for these
as an FYI.

Otherwise, I'll let you take lib/ tests which depend on / conflict
with the renames.

Cheers,
-- David


>
> Bruno Sobreira França (1):
>   lib/math: Add int_log test suite
>
> Diego Vieira (1):
>   lib/tests/kfifo_kunit.c: add tests for the kfifo structure
>
> Gabriela Bittencourt (2):
>   unicode: kunit: refactor selftest to kunit tests
>   unicode: kunit: change tests filename and path
>
> Kees Cook (1):
>   lib: Move KUnit tests into tests/ subdirectory
>
> Luis Felipe Hernandez (1):
>   lib: math: Move KUnit tests into tests/ subdir
>
>  MAINTAINERS   |  19 +-
>  fs/unicode/Kconfig|   5 +-
>  fs/unicode/Makefile   |   2 +-
>  fs/unicode/tests/.kunitconfig |   3 +
>  .../{utf8-selftest.c => tests/utf8_kunit.c}   | 149 ++--
>  fs/unicode/utf8-norm.c|   2 +-
>  lib/Kconfig.debug |  27 ++-
>  lib/Makefile  |  39 +--
>  lib/math/Makefile |   5 +-
>  lib/math/tests/Makefile   |   4 +-
>  lib/math/tests/int_log_kunit.c|  74 ++
>  .../rational_kunit.c} |   0
>  lib/tests/Makefile|  41 
>  lib/{ => tests}/bitfield_kunit.c  |   0
>  lib/{ => tests}/checksum_kunit.c  |   0
>  lib/{ => tests}/cmdline_kunit.c   |   0
>  lib/{ => tests}/cpumask_kunit.c   |   0
>  lib/{ => tests}/crc_kunit.c   |   0
>  lib/{ => tests}/fortify_kunit.c   |   0
>  lib/{ => tests}/hashtable_test.c  |   0
>  lib/{ => tests}/is_signed_type_kunit.c|   0
>  lib/tests/kfifo_kunit.c   | 224 ++
>  lib/{ => tests}/kunit_iov_iter.c  |   0
>  lib/{ => tests}/list-test.c   |   0
>  lib/{ => tests}/memcpy_kunit.c|   0
>  lib/{ => tests}/overflow_kunit.c  |   0
>  lib/{ => tests}/siphash_kunit.c   |   0
>  lib/{ => tests}/slub_kunit.c  |   0
>  lib/{ => tests}/stackinit_kunit.c |   0
>  lib/{ => tests}/string_helpers_kunit.c|   0
>  lib/{ => tests}/string_kunit.c|   0
>  lib/{ => tests}/test_bits.c   |   0
>  lib/{ => tests}/test_fprobe.c |   0
>  lib/{ => tests}/test_hash.c   |   0
>  lib/{ => tests}/test_kprobes.c|   0
>  lib/{ => tests}/test_linear_ranges.c  |   0
>  lib/{ => tests}/test_list_sort.c  |   0
>  lib/{ => tests}/test_sort.c   |   0
>  lib/{ => tests}/usercopy_kunit.c  |   0
>  lib/{ => tests}/util_macros_kunit.c   |   0
>  40 files changed, 458 insertions(+), 136 deletions(-)
>  create mode 100644 fs/unicode/tests/.kunitconfig
>  rename fs/unicode/{utf8-selftest.c => tests/utf8_kunit.c} (64%)
>  create mode 100644 lib/math/tests/int_log_kunit.c
>  rename lib/math/{rational-test.c => tests/rational_kunit.c} (100%)
>  rename lib/{ => tests}/bitfield_kunit.c (100%)
>  rename lib/{ => tests}/checksum_kunit.c (100%)
>  rename lib/{ => tests}/cmdline_kunit.c (100%)
>  rename lib/{ => tests}/cpumask_kunit.c (100%)
>  rename lib/{ => tests}/crc_kunit.c (100%)
>  rename lib/{ => tests}/fortify_kunit.c (100%)
>  rename lib/{ => tests}/hashtable_test.c (100%)
>  rename lib/{ => tests}/is_signed_type_kunit.c (100%)
>  create mode 100644 lib/tests/kfifo_kunit.c
>  rename lib/{ => tests}/kunit_iov_iter.c (100%)
>  rename lib/{ => tests}/list-test.c (100%)
>  rename lib/{ => tests}/memcpy_kunit.c (100%)
>  rename lib/{ => tests}/overflow_kunit.c (100%)
>  rename lib/{ => tests}/siphash_kunit.c (100%)
>  rename lib/{ => tests}/slub_kunit.c (100%)
>  rename lib/{ => tests}/stackinit_kunit.c (100%)
>  rename lib/{ => tests}/string_helpers_kunit.c (100%)
>  rename lib/{ => tests}/string_kunit.c (100%)
>  rename lib/{ => tests}/test_bits.c (100%)
>  rename lib/{ => tests}/test_fprobe.c (100%)
>  rename lib/{ => tests}/test_hash.c (100%)
>  rename lib/{ => tests}/test_kprobes.c (100%)
>  rename lib/{ => tests}/test_linear_ranges.c (100%)
>  rename lib/{ => tests}/test_list_sort.c (100%)
>  rename lib/{ => tests}/test_sort.c (100%)
>  rename lib/{ => tests}/usercopy_kunit.c (100%)
>  rename lib/{ => tests}/util_m

Re: [PATCH] octeontx2-af: Fix uninitialized scalar variable

2025-02-10 Thread Michal Swiatkowski
On Mon, Feb 10, 2025 at 09:01:52PM -0500, Ethan Carter Edwards wrote:
> The variable *max_mtu* is uninitialized in the function
> otx2_get_max_mtu. It is only assigned in the if-statement, leaving the
> possibility of returning an uninitialized value.

In which case? If rc == 0 at the end of the function max_mtu is set to
custom value from HW. If rc != it will reach the if after goto label and
set max_mtu to default.

In my opinion this change is good. It is easier to see that the variable
is alwyas correct initialized, but I don't think it is a fix for real
issue.

Thanks,
Michal

> 
> 1500 is the industry standard networking mtu and therefore should be the
> default. If the function detects that the hardware custom sets the mtu,
> then it will use it instead.
> 
> Addresses-Coverity-ID: 1636407 ("Uninitialized scalar variable")
> Fixes: ab58a416c93f ("octeontx2-pf: cn10k: Get max mtu supported from admin 
> function")
> Signed-off-by: Ethan Carter Edwards 
> ---
>  drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c 
> b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
> index 
> 2b49bfec78692cf1f63c793ec49511607cda7c3e..6c1b03690a9c24c5232ff9f07befb1cc553490f7
>  100644
> --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
> @@ -1909,7 +1909,7 @@ u16 otx2_get_max_mtu(struct otx2_nic *pfvf)
>  {
>   struct nix_hw_info *rsp;
>   struct msg_req *req;
> - u16 max_mtu;
> + u16 max_mtu = 1500;
>   int rc;
>  
>   mutex_lock(&pfvf->mbox.lock);
> @@ -1948,7 +1948,6 @@ u16 otx2_get_max_mtu(struct otx2_nic *pfvf)
>   if (rc) {
>   dev_warn(pfvf->dev,
>"Failed to get MTU from hardware setting default 
> value(1500)\n");
> - max_mtu = 1500;
>   }
>   return max_mtu;
>  }
> 
> ---
> base-commit: febbc555cf0fff895546ddb8ba2c9a523692fb55
> change-id: 20250210-otx2_common-453132aa0a24
> 
> Best regards,
> -- 
> Ethan Carter Edwards 



[PATCH] ASoC: Intel: sof_sdw: initialize ret in asoc_sdw_rt_amp_spk_rtd_init()

2025-02-10 Thread Ethan Carter Edwards
There is a possibility for an uninitialized *ret* variable to be
returned in some code paths.

Setting to 0 prevents a random value from being returned.

Closes: 
https://scan5.scan.coverity.com/#/project-view/63873/10063?selectedIssue=1627120
Signed-off-by: Ethan Carter Edwards 
---
Would it potentially be better to remove ret entirely and just return 0
explicitly at the end of the function and directly return in the if
statements? I'm not sure.
---
 sound/soc/sdw_utils/soc_sdw_rt_amp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/sdw_utils/soc_sdw_rt_amp.c 
b/sound/soc/sdw_utils/soc_sdw_rt_amp.c
index 
0538c252ba69b1f5a24ba2de2a610b22d0c0665f..61a00a3548d85689c13e2d2a301da17a572e4a0e
 100644
--- a/sound/soc/sdw_utils/soc_sdw_rt_amp.c
+++ b/sound/soc/sdw_utils/soc_sdw_rt_amp.c
@@ -190,7 +190,7 @@ int asoc_sdw_rt_amp_spk_rtd_init(struct snd_soc_pcm_runtime 
*rtd, struct snd_soc
const struct snd_soc_dapm_route *rt_amp_map;
char codec_name[CODEC_NAME_SIZE];
struct snd_soc_dai *codec_dai;
-   int ret;
+   int ret = 0;
int i;
 
rt_amp_map = get_codec_name_and_route(dai, codec_name);

---
base-commit: febbc555cf0fff895546ddb8ba2c9a523692fb55
change-id: 20250210-soc_sdw_rt_amp-e9c703ebe4dc

Best regards,
-- 
Ethan Carter Edwards 




Re: [PATCH] wifi: ath12k: Fix uninitialized variable and remove goto

2025-02-10 Thread Jeff Johnson
On 2/9/2025 8:36 PM, Ethan Carter Edwards wrote:

commit subject should be as specific as possible.
suggest you at least mention the function

> There is a possibility for an uninitialized *ret* variable to be
> returned in some code paths.
> 
> This moves *ret* inside the conditional and explicitly returns 0 without
> an error. Also removes goto that returned *ret*.

ath driver convention is to declare function variables at the beginning of the
function -- please do not relocate the definition of 'ret'

> 
> Addresses-Coverity-ID: 1642337 ("Uninitialized scalar variable")

Is that an official kernel tag? IMO the proper tag would be

Closes: 
https://scan5.scan.coverity.com/#/project-view/63541/10063?selectedIssue=1642337

(is there a shorter URL?)

see 
https://www.kernel.org/doc/html/latest/process/submitting-patches.html#using-reported-by-tested-by-reviewed-by-suggested-by-and-fixes

> Fixes: b716a10d99a28 ("wifi: ath12k: enable MLO setup and teardown from core")
> Signed-off-by: Ethan Carter Edwards 
> ---
>  drivers/net/wireless/ath/ath12k/core.c | 8 +++-
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath12k/core.c 
> b/drivers/net/wireless/ath/ath12k/core.c
> index 
> 0606116d6b9c491b6ede401b2e1aedfb619339a8..ae75cdad3bd6b6eb80a35fee94c18d365d123cbd
>  100644
> --- a/drivers/net/wireless/ath/ath12k/core.c
> +++ b/drivers/net/wireless/ath/ath12k/core.c
> @@ -908,7 +908,6 @@ int ath12k_mac_mlo_ready(struct ath12k_hw_group *ag)
>  {
>   struct ath12k_hw *ah;
>   struct ath12k *ar;
> - int ret;
>   int i, j;
>  
>   for (i = 0; i < ag->num_hw; i++) {
> @@ -918,14 +917,13 @@ int ath12k_mac_mlo_ready(struct ath12k_hw_group *ag)
>  
>   for_each_ar(ah, ar, j) {
>   ar = &ah->radio[j];
> - ret = __ath12k_mac_mlo_ready(ar);
> + int ret = __ath12k_mac_mlo_ready(ar);
>   if (ret)
> - goto out;
> + return ret;
>   }
>   }
>  
> -out:
> - return ret;
> + return 0;
>  }
>  
>  static int ath12k_core_mlo_setup(struct ath12k_hw_group *ag)
> 
> ---
> base-commit: a64dcfb451e254085a7daee5fe51bf22959d52d3
> change-id: 20250209-ath12k-uninit-18560fd91c07
> 
> Best regards,

replacing goto out with return ret and unconditional return 0 LGTM.

can you respin a v2 with the other comments addressed?

/jeff



Re: [PATCH v2 0/6] KUnit test moves / renames

2025-02-10 Thread Andrew Morton
On Mon, 10 Feb 2025 16:31:28 -0800 Kees Cook  wrote:

> This is rebased to v6.14-rc2. I'll carry it and folks can send me new
> tests, etc, as needed to minimize future collisions.

Actually seems designed to maximize collisions.

I'll drop ("lib/math: add Kunit test suite for gcd()")
https://lore.kernel.org/all/0eaf6b69aeb2fe35092a633fed12537efe645303.1727332572.git.zhengqi.a...@bytedance.com/T/#u.