xarray, fault injection and syzkaller

[Jason Gunthorpe]

Hi All,

I wonder if anyone has some thoughts on this - I have spent some time
setting up syzkaller for a new subsystem and I've noticed that nth
fault injection does not reliably cause things like xa_store() to
fail.

It seems the basic reason is that xarray will usually do two
allocations, one in an atomic context which fault injection does
reliably fail, but then it almost always follows up with a second
allocation in a non-atomic context that doesn't fail because nth has
become 0.

This reduces the available coverage that syzkaller can achieve by
randomizing fault injections. It does very rarely provoke a failure,
which I guess is because the atomic allocation fails naturally
sometimes with low probability and the nth takes out the non-atomic
allocation.. But it is rare and very annoying to reproduce.

Does anyone have some thoughts on what is an appropriate way to cope
with this? It seems like some sort of general problem with these sorts
of fallback allocations, so perhaps a GFP_ flag of some sort that
causes allows the fault injection to fail but not decrement nth?

Thanks,
Jason

[Matthew Wilcox]

On Thu, Nov 03, 2022 at 04:09:04PM -0300, Jason Gunthorpe wrote:
> Hi All,
>
> I wonder if anyone has some thoughts on this - I have spent some time
> setting up syzkaller for a new subsystem and I've noticed that nth
> fault injection does not reliably cause things like xa_store() to
> fail.
>
> It seems the basic reason is that xarray will usually do two
> allocations, one in an atomic context which fault injection does
> reliably fail, but then it almost always follows up with a second
> allocation in a non-atomic context that doesn't fail because nth has
> become 0.

Hahaha. I didn't intentionally set out to thwart memory allocation
fault injection. Realistically, do we want it to fail though?
GFP_KERNEL allocations of small sizes are supposed to never fail.
(for those not aware, node allocations are 576 bytes; typically the slab
allocator bundles 28 of them into an order-2 allocation).

I think a simple solution if we really do want to make allocations fail
is to switch error injection from "fail one allocation per N" to "fail
M allocations per N". eg, 7 allocations succeed, 3 allocations fail,
7 succeed, 3 fail, ... It's more realistic because you do tend to see
memory allocation failures come in bursts.

[Jason Gunthorpe]

On Thu, Nov 03, 2022 at 08:00:25PM +0000, Matthew Wilcox wrote:
> On Thu, Nov 03, 2022 at 04:09:04PM -0300, Jason Gunthorpe wrote:
> > Hi All,
> >
> > I wonder if anyone has some thoughts on this - I have spent some time
> > setting up syzkaller for a new subsystem and I've noticed that nth
> > fault injection does not reliably cause things like xa_store() to
> > fail.
> >
> > It seems the basic reason is that xarray will usually do two
> > allocations, one in an atomic context which fault injection does
> > reliably fail, but then it almost always follows up with a second
> > allocation in a non-atomic context that doesn't fail because nth has
> > become 0.
>
> Hahaha. I didn't intentionally set out to thwart memory allocation
> fault injection. Realistically, do we want it to fail though?
> GFP_KERNEL allocations of small sizes are supposed to never fail.
> (for those not aware, node allocations are 576 bytes; typically the slab
> allocator bundles 28 of them into an order-2 allocation).

I don't know, I have code to handle these failures, I want to test it
:)

> I think a simple solution if we really do want to make allocations fail
> is to switch error injection from "fail one allocation per N" to "fail
> M allocations per N". eg, 7 allocations succeed, 3 allocations fail,
> 7 succeed, 3 fail, ... It's more realistic because you do tend to see
> memory allocation failures come in bursts.

The systemic testing I've setup just walks nth through the entire
range until it no longer triggers. This hits every injection point and
checks the failure path of it. This is also what syzkaller does
automatically from what I can tell

If we make it probabilistic it is harder to reliably trigger these
fault points.

Jason

[Dmitry Vyukov]

On Thu, 3 Nov 2022 at 13:07, 'Jason Gunthorpe' via syzkaller-bugs
<syzkall...@googlegroups.com> wrote:
>
> On Thu, Nov 03, 2022 at 08:00:25PM +0000, Matthew Wilcox wrote:
> > On Thu, Nov 03, 2022 at 04:09:04PM -0300, Jason Gunthorpe wrote:
> > > Hi All,
> > >
> > > I wonder if anyone has some thoughts on this - I have spent some time
> > > setting up syzkaller for a new subsystem and I've noticed that nth
> > > fault injection does not reliably cause things like xa_store() to
> > > fail.

Hi Jason, Matthew,

Interesting. Where exactly is that kmalloc sequence? xa_store() itself
does not have any allocations:
https://elixir.bootlin.com/linux/v6.1-rc3/source/lib/xarray.c#L1577

Do we know how common/useful such an allocation pattern is?
If it's common/useful, then it can be turned into a single kmalloc()
with some special flag that will try both allocation modes at once.

Potentially fail-nth interface can be extended to accept a set of
sites, e.g. "5,7" or, "5-100".
Though, not sure what the systematic enumeration should be then if we
go beyond "every single site on its own"... but I guess we can figure
it out.

> > > It seems the basic reason is that xarray will usually do two
> > > allocations, one in an atomic context which fault injection does
> > > reliably fail, but then it almost always follows up with a second
> > > allocation in a non-atomic context that doesn't fail because nth has
> > > become 0.
> >
> > Hahaha. I didn't intentionally set out to thwart memory allocation
> > fault injection. Realistically, do we want it to fail though?
> > GFP_KERNEL allocations of small sizes are supposed to never fail.
> > (for those not aware, node allocations are 576 bytes; typically the slab
> > allocator bundles 28 of them into an order-2 allocation).

I hear this statement periodically. But I can't understand its status.
We discussed it recently here:
https://lore.kernel.org/all/CACT4Y+Y_kg1J00iBL=sMr5AP7U4RXuBizu...@mail.gmail.com/

Likely/unlikely is not what matters, right? It's only: can it fail at
all or not?
If some allocation can't fail 100% and we want to rely on this in
future (not just a current implementation detail), then I think we
need to (1) make fault injection to not fail them, add a BUG_ON in the
allocation to panic if they do fail, (3) maybe start removing error
handling code (since having buggy/untested/confusing code is not
useful).

[Jason Gunthorpe]

On Thu, Nov 03, 2022 at 05:11:04PM -0700, Dmitry Vyukov wrote:
> On Thu, 3 Nov 2022 at 13:07, 'Jason Gunthorpe' via syzkaller-bugs
> <syzkall...@googlegroups.com> wrote:
> >
> > On Thu, Nov 03, 2022 at 08:00:25PM +0000, Matthew Wilcox wrote:
> > > On Thu, Nov 03, 2022 at 04:09:04PM -0300, Jason Gunthorpe wrote:
> > > > Hi All,
> > > >
> > > > I wonder if anyone has some thoughts on this - I have spent some time
> > > > setting up syzkaller for a new subsystem and I've noticed that nth
> > > > fault injection does not reliably cause things like xa_store() to
> > > > fail.
>
> Hi Jason, Matthew,
>
> Interesting. Where exactly is that kmalloc sequence? xa_store() itself
> does not have any allocations:
> https://elixir.bootlin.com/linux/v6.1-rc3/source/lib/xarray.c#L1577

The first effort is this call chain

__xa_store()
xas_store()
xas_create()
xas_alloc()
kmem_cache_alloc_lru(GFP_NOWAIT | __GFP_NOWARN)

If that fails then __xa_store() will do:

__xa_store()
__xas_nomem()
xas_unlock_type(xas, lock_type);
kmem_cache_alloc_lru(GFP_KERNEL);
xas_lock_type(xas, lock_type);

They key point being that the retry is structured in a way that allows
dropping the spinlocks that are forcing the first allocation to be
atomic.

> Do we know how common/useful such an allocation pattern is?

I have coded something like this a few times, in my cases it is
usually something like: try to allocate a big chunk of memory hoping
for a huge page, then fall back to a smaller allocation

Most likely the key consideration is that the callsites are using
GFP_NOWARN, so perhaps we can just avoid decrementing the nth on a
NOWARN case assuming that another allocation attempt will closely
follow?

> If it's common/useful, then it can be turned into a single kmalloc()
> with some special flag that will try both allocation modes at once.

A single call doesn't really suit the use cases..

> Potentially fail-nth interface can be extended to accept a set of
> sites, e.g. "5,7" or, "5-100".

For my purposes this is possibly Ok, you'd just set N->large and step
N to naively cover the error paths.

However, this would also have to fix the obnoxious behavior of fail
nth where it fails its own copy_from_user on its write system call -
meaning there would be no way to turn it off.

> > > Hahaha. I didn't intentionally set out to thwart memory allocation
> > > fault injection. Realistically, do we want it to fail though?
> > > GFP_KERNEL allocations of small sizes are supposed to never fail.
> > > (for those not aware, node allocations are 576 bytes; typically the slab
> > > allocator bundles 28 of them into an order-2 allocation).
>
> I hear this statement periodically. But I can't understand its
> status. We discussed it recently here:

I was thinking about this after, and at least for what I am doing it
doesn't apply. All the allocations here are GFP_KERNEL_ACCOUNT and the
cgroup can definitely reject any allocation at any time even if it is
"small"

So I can't ignore allocation failures as something that is unlikely. A
hostile userspace contained in a cgroup sandbox can reliably trigger
them at will.

Jason

[Dmitry Vyukov]

iOn Thu, 3 Nov 2022 at 17:21, 'Jason Gunthorpe' via syzkaller-bugs

I see. Yes, as you note below, this cannot be folded into a single
allocation call.

> > Do we know how common/useful such an allocation pattern is?
>
> I have coded something like this a few times, in my cases it is
> usually something like: try to allocate a big chunk of memory hoping
> for a huge page, then fall back to a smaller allocation
>
> Most likely the key consideration is that the callsites are using
> GFP_NOWARN, so perhaps we can just avoid decrementing the nth on a
> NOWARN case assuming that another allocation attempt will closely
> follow?

GFP_NOWARN is also extensively used for allocations with
user-controlled size, e.g.:
https://elixir.bootlin.com/linux/v6.1-rc3/source/net/unix/af_unix.c#L3451

That's different and these allocations are usually not repeated.
So looking at GFP_NOWARN does not look like the right thing to do.

> > If it's common/useful, then it can be turned into a single kmalloc()
> > with some special flag that will try both allocation modes at once.
>
> A single call doesn't really suit the use cases..
>
> > Potentially fail-nth interface can be extended to accept a set of
> > sites, e.g. "5,7" or, "5-100".
>
> For my purposes this is possibly Ok, you'd just set N->large and step
> N to naively cover the error paths.

Filed https://bugzilla.kernel.org/show_bug.cgi?id=216661 for this.

> However, this would also have to fix the obnoxious behavior of fail
> nth where it fails its own copy_from_user on its write system call -
> meaning there would be no way to turn it off.

Oh, interesting. We added failing of copy_from/to_user later and did
not consider such interaction.
Filed https://bugzilla.kernel.org/show_bug.cgi?id=216660 for this.

> > > > Hahaha. I didn't intentionally set out to thwart memory allocation
> > > > fault injection. Realistically, do we want it to fail though?
> > > > GFP_KERNEL allocations of small sizes are supposed to never fail.
> > > > (for those not aware, node allocations are 576 bytes; typically the slab
> > > > allocator bundles 28 of them into an order-2 allocation).
> >
> > I hear this statement periodically. But I can't understand its
> > status. We discussed it recently here:
>
> I was thinking about this after, and at least for what I am doing it
> doesn't apply. All the allocations here are GFP_KERNEL_ACCOUNT and the
> cgroup can definitely reject any allocation at any time even if it is
> "small"
>
> So I can't ignore allocation failures as something that is unlikely. A
> hostile userspace contained in a cgroup sandbox can reliably trigger
> them at will.
>
> Jason
>

> --
> You received this message because you are subscribed to the Google Groups "syzkaller-bugs" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to syzkaller-bug...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/syzkaller-bugs/Y2RbCUdEY2syxRLW%40nvidia.com.

[Jason Gunthorpe]

On Fri, Nov 04, 2022 at 10:47:17AM -0700, Dmitry Vyukov wrote:
> > > Do we know how common/useful such an allocation pattern is?
> >
> > I have coded something like this a few times, in my cases it is
> > usually something like: try to allocate a big chunk of memory hoping
> > for a huge page, then fall back to a smaller allocation
> >
> > Most likely the key consideration is that the callsites are using
> > GFP_NOWARN, so perhaps we can just avoid decrementing the nth on a
> > NOWARN case assuming that another allocation attempt will closely
> > follow?
>
> GFP_NOWARN is also extensively used for allocations with
> user-controlled size, e.g.:
> https://elixir.bootlin.com/linux/v6.1-rc3/source/net/unix/af_unix.c#L3451
>
> That's different and these allocations are usually not repeated.
> So looking at GFP_NOWARN does not look like the right thing to do.

This may be the best option then, arguably perhaps even more
"realistic" than normal fail_nth as in a real system if this stuff
starts failing there is a good chance things from then on will fail
too during the error cleanup.

> > However, this would also have to fix the obnoxious behavior of fail
> > nth where it fails its own copy_from_user on its write system call -
> > meaning there would be no way to turn it off.
>
> Oh, interesting. We added failing of copy_from/to_user later and did
> not consider such interaction.
> Filed https://bugzilla.kernel.org/show_bug.cgi?id=216660 for this.

Oh, I will tell you the other two bugish things I noticed

__should_failslab() has this:

if (gfpflags & __GFP_NOWARN)
failslab.attr.no_warn = true;

return should_fail(&failslab.attr, s->object_size);

Which always permanently turns off no_warn for slab during early
boot. This is why syzkaller reports are so confusing. They trigger a
slab fault injection, which in all other cases gives a notification
backtrace, but in slab cases there is no hint about the fault
injection in the log.

Once that is fixed we can quickly explain why the socketpair() example
in the docs shows success ret codes in the middle of the sweep when
run on syzkaller kernels

fail_nth interacts badly with other kernel features typically enabled
in syzkaller kernels. Eg it fails in hidden kmemleak instrumentation:

[ 18.499559] FAULT_INJECTION: forcing a failure.
[ 18.499559] name failslab, interval 1, probability 0, space 0, times 0
[ 18.499720] CPU: 10 PID: 386 Comm: iommufd_fail_nt Not tainted 6.1.0-rc3+ #34
[ 18.499826] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
[ 18.499971] Call Trace:
[ 18.500010] <TASK>
[ 18.500048] show_stack+0x3d/0x3f
[ 18.500114] dump_stack_lvl+0x92/0xbd
[ 18.500171] dump_stack+0x15/0x17
[ 18.500232] should_fail.cold+0x5/0xa
[ 18.500291] __should_failslab+0xb6/0x100
[ 18.500349] should_failslab+0x9/0x20
[ 18.500416] kmem_cache_alloc+0x64/0x4e0
[ 18.500477] ? __create_object+0x40/0xc50
[ 18.500539] __create_object+0x40/0xc50
[ 18.500620] ? kasan_poison+0x3a/0x50
[ 18.500690] ? kasan_unpoison+0x28/0x50
[***18.500753] kmemleak_alloc+0x24/0x30
[ 18.500816] __kmem_cache_alloc_node+0x1de/0x400
[ 18.500900] ? iopt_alloc_area_pages+0x95/0x560 [iommufd]
[ 18.500993] kmalloc_trace+0x26/0x110
[ 18.501059] iopt_alloc_area_pages+0x95/0x560 [iommufd]

Which has the consequence of syzkaller wasting half its fail_nth
effort because it is triggering failures in hidden instrumentation
that has no impact on the main code path.

Maybe a kmem_cache_alloc_no_fault_inject() would be helpful for a few
cases.

Jason

[Dmitry Vyukov]

Ouch, this looks like a bug in:

commit 3f913fc5f9745613088d3c569778c9813ab9c129
Author: Qi Zheng <zhengq...@bytedance.com>
Date: Thu May 19 14:08:55 2022 -0700
mm: fix missing handler for __GFP_NOWARN

+Qi could you please fix it?

At the very least the local gfpflags should not alter the global
failslab.attr that is persistent and shared by all tasks.

But I am not sure if we really don't want to issue the fault injection
stack in this case. It's not a WARNING, it's merely an information
message. It looks useful in all cases, even with GFP_NOWARN. Why
should it be suppressed?

[Jason Gunthorpe]

On Fri, Nov 04, 2022 at 11:21:21AM -0700, Dmitry Vyukov wrote:

> But I am not sure if we really don't want to issue the fault injection
> stack in this case. It's not a WARNING, it's merely an information
> message. It looks useful in all cases, even with GFP_NOWARN. Why
> should it be suppressed?

I think it is fine to suppress it for *this call* but the bug turns it
off forever more

Jason

[Dmitry Vyukov]

Is it just "fine", or "good"? I agree it's probably "fine", but
wouldn't it be better to not suppress it?

The message fault injection prints is not a warning, and the
allocation failed due to fault injection. That may trigger subsequent
bugs just as any other case of fault injection. Why don't we want to
see the info message in this particular case? NOWARN looks orthogonal
to this, it's about normal slab allocation failures.

[Dmitry Vyukov]

On Fri, 4 Nov 2022 at 11:03, Jason Gunthorpe <j...@nvidia.com> wrote:
>

Filed https://bugzilla.kernel.org/show_bug.cgi?id=216663
We could add GFP_NOFAULT. But now I am thinking if using the existing
GFP_NOFAIL is actually the right thing to do in these cases...
(details are in the bug).

[Jason Gunthorpe]

On Fri, Nov 04, 2022 at 11:38:48AM -0700, Dmitry Vyukov wrote:
> On Fri, 4 Nov 2022 at 11:30, Jason Gunthorpe <j...@nvidia.com> wrote:
> > On Fri, Nov 04, 2022 at 11:21:21AM -0700, Dmitry Vyukov wrote:
> >
> > > But I am not sure if we really don't want to issue the fault injection
> > > stack in this case. It's not a WARNING, it's merely an information
> > > message. It looks useful in all cases, even with GFP_NOWARN. Why
> > > should it be suppressed?
> >
> > I think it is fine to suppress it for *this call* but the bug turns it
> > off forever more
>
> Is it just "fine", or "good"? I agree it's probably "fine", but
> wouldn't it be better to not suppress it?

It seems sensible to me that fail*/verbosity=2 should *always* print
if a fault has been injected.

I don't know why someone thought this one deserves to be shut down.

GFP_NOWARN is about the caller indicating that it expects and will
handle an allocation failure (eg it is asking for big regions and has
fallbacks) so we shouldn't print the general OOM warning.

Jason

[Qi Zheng]

Oh, It indeed shouldn't alter the global failslab.attr, I'll fix it.

But a warning should not be printed for callers that currently specify
__GFP_NOWARN, because that could lead to deadlocks, such as the deadlock
case mentioned in commit 6b9dbedbe349 ("tty: fix deadlock caused by
calling printk() under tty_port->lock").

Thanks,
Qi

--
Thanks,
Qi

[Qi Zheng]

How about the following changes? If it's ok, I will send this fix patch.
Thanks. :)

diff --git a/include/linux/fault-inject.h b/include/linux/fault-inject.h
index 9f6e25467844..b61a3fb7a2a3 100644
--- a/include/linux/fault-inject.h
+++ b/include/linux/fault-inject.h
@@ -20,7 +20,6 @@ struct fault_attr {
atomic_t space;
unsigned long verbose;
bool task_filter;
- bool no_warn;
unsigned long stacktrace_depth;
unsigned long require_start;
unsigned long require_end;
@@ -40,12 +39,12 @@ struct fault_attr {
.ratelimit_state = RATELIMIT_STATE_INIT_DISABLED, \
.verbose = 2, \
.dname = NULL, \
- .no_warn = false, \
}

#define DECLARE_FAULT_ATTR(name) struct fault_attr name =
FAULT_ATTR_INITIALIZER
int setup_fault_attr(struct fault_attr *attr, char *str);
bool should_fail(struct fault_attr *attr, ssize_t size);
+bool should_fail_gfp(struct fault_attr *attr, ssize_t size, gfp_t
gfpflags);

#ifdef CONFIG_FAULT_INJECTION_DEBUG_FS

diff --git a/lib/fault-inject.c b/lib/fault-inject.c
index 4b8fafce415c..95af50832770 100644
--- a/lib/fault-inject.c
+++ b/lib/fault-inject.c
@@ -41,9 +41,6 @@ EXPORT_SYMBOL_GPL(setup_fault_attr);

static void fail_dump(struct fault_attr *attr)
{
- if (attr->no_warn)
- return;
-
if (attr->verbose > 0 && __ratelimit(&attr->ratelimit_state)) {
printk(KERN_NOTICE "FAULT_INJECTION: forcing a failure.\n"
"name %pd, interval %lu, probability %lu, "
@@ -98,12 +95,7 @@ static inline bool fail_stacktrace(struct fault_attr
*attr)

#endif /* CONFIG_FAULT_INJECTION_STACKTRACE_FILTER */

-/*
- * This code is stolen from failmalloc-1.0
- * http://www.nongnu.org/failmalloc/
- */
-
-bool should_fail(struct fault_attr *attr, ssize_t size)
+bool should_fail_check(struct fault_attr *attr, ssize_t size)
{
bool stack_checked = false;

@@ -118,7 +110,7 @@ bool should_fail(struct fault_attr *attr, ssize_t size)
fail_nth--;
WRITE_ONCE(current->fail_nth, fail_nth);
if (!fail_nth)
- goto fail;
+ return true;

return false;
}
@@ -151,7 +143,19 @@ bool should_fail(struct fault_attr *attr, ssize_t size)
if (attr->probability <= get_random_u32_below(100))
return false;

-fail:
+ return true;
+}
+
+/*
+ * This code is stolen from failmalloc-1.0
+ * http://www.nongnu.org/failmalloc/
+ */
+
+bool should_fail(struct fault_attr *attr, ssize_t size)
+{
+ if (!should_fail_check(attr, size))
+ return false;
+
fail_dump(attr);

if (atomic_read(&attr->times) != -1)
@@ -161,6 +165,21 @@ bool should_fail(struct fault_attr *attr, ssize_t size)
}
EXPORT_SYMBOL_GPL(should_fail);

+bool should_fail_gfp(struct fault_attr *attr, ssize_t size, gfp_t gfpflags)
+{
+ if (!should_fail_check(attr, size))
+ return false;
+
+ if (!(gfpflags & __GFP_NOWARN))
+ fail_dump(attr);
+
+ if (atomic_read(&attr->times) != -1)
+ atomic_dec_not_zero(&attr->times);
+
+ return true;
+}
+EXPORT_SYMBOL_GPL(should_fail_gfp);
+
#ifdef CONFIG_FAULT_INJECTION_DEBUG_FS

static int debugfs_ul_set(void *data, u64 val)
diff --git a/mm/failslab.c b/mm/failslab.c
index 58df9789f1d2..21338b256791 100644
--- a/mm/failslab.c
+++ b/mm/failslab.c
@@ -30,10 +30,7 @@ bool __should_failslab(struct kmem_cache *s, gfp_t
gfpflags)
if (failslab.cache_filter && !(s->flags & SLAB_FAILSLAB))
return false;

- if (gfpflags & __GFP_NOWARN)
- failslab.attr.no_warn = true;
-
- return should_fail(&failslab.attr, s->object_size);
+ return should_fail_gfp(&failslab.attr, s->object_size, gfpflags);
}

static int __init setup_failslab(char *str)
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 7192ded44ad0..4e70b5599ada 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -3912,10 +3912,7 @@ static bool __should_fail_alloc_page(gfp_t
gfp_mask, unsigned int order)
(gfp_mask & __GFP_DIRECT_RECLAIM))
return false;

- if (gfp_mask & __GFP_NOWARN)
- fail_page_alloc.attr.no_warn = true;
-
- return should_fail(&fail_page_alloc.attr, 1 << order);
+ return should_fail_gfp(&fail_page_alloc.attr, 1 << order, gfp_mask);
}

#ifdef CONFIG_FAULT_INJECTION_DEBUG_FS

[Dmitry Vyukov]

I think the interface design question is mostly to Akinobu as fault
injection maintainer.

should_fail/should_fail_gfp duplicate some code + gfp is slab-specific
(while clumsy to use for other fault injection types + we won't be
able to pass any runtime flags that are not present in gfp). So I
would go either with:

bool should_fail(struct fault_attr *attr, ssize_t size, bool nowarn);

or even more extensible interface would be:

enum fault_flags {
fault_nowarn = 1 << 0,
};

bool should_fail(struct fault_attr *attr, ssize_t size, fault_flags flags);

And if we don't want to change all callers to avoid code duplication:

bool should_fail_ex(struct fault_attr *attr, ssize_t size, fault_flags flags);
bool should_fail(struct fault_attr *attr, ssize_t size) {
return should_fail_ex(attr, size, 0);

> --
> You received this message because you are subscribed to the Google Groups "syzkaller-bugs" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to syzkaller-bug...@googlegroups.com.

> To view this discussion on the web visit https://groups.google.com/d/msgid/syzkaller-bugs/be6a67b0-479f-db0a-fa69-764713135d70%40bytedance.com.

[Qi Zheng]

This looks better to me, I will revise and resend this fix patch later.

Thanks,
Qi

--
Thanks,
Qi