lkml.org 
[lkml]   [2021]   [Nov]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH] vt: Fix sleeping functions called from atomic context
Date
On Wednesday, November 17, 2021 2:55:44 AM CET Tetsuo Handa wrote:
> On 2021/11/16 23:49, Fabio M. De Francesco wrote:
> > - if (in_interrupt())
> > + if (!preemptible())
> > return count;
>
> preemptible() is "an unconditional 0" if CONFIG_PREEMPT_COUNT=n .
> Is preemptible() really what we want? ;-)
>
Greg K-H made me notice that Linux has had this code for many years and no
one has ever found problems with it, therefore, whatever triggered Syzbot
must be recent changes up in the calls chain.

This reported bug must be better investigated because there is a high
probability that the "real" issue is somewhere else.

Unfortunately, as I replied to Greg, I know very little of tty/vt so I'll
take time to understand the relevant documentation and the related code.

For instance, I don't know why we need to disable IRQs with spin_lock_irq()
up in the call chain in n_tty_ioctl_helper(). Would a "normal" spin_lock() be
enough? Otherwise, what about spin_lock_bh()?

Do we really need to disable IRQs?

Aside from the reasoning reported above, for a moment let's speculate on the
remote possibility that this bugs are in do_con_write() and
con_flush_chars()...

When I asked Syzbot to run a test on my diff, the code was different than
what you see in this patch.

Marco Elver had proposed this "if (!preemptible()) return <something>;". I
didn't even know of that macro, so I proposed to open code the equivalent
implementation that Linux uses if CONFIG_PREEMPT_COUNT=y.

The end result, applying De Morgan's law on his proposal, was "if
((preempt_count() || irqs_disabled()) return <something>;".

Now, out of curiosity, if Greg's argument (with which I agree in full) was
not brought to my attention, what would happen by using that test if we have
kernels compiled with CONFIG_PREEMPT_COUNT=n (as it is in your example)?

I'd appreciate if you have time and want to answer my last question and
(possibly) also the one about using spin_lock() or spin_lock_bh() in
n_tty_ioctl_helper().

@Greg, the same two questions are for you, if you have time and want to
kindly help :)

Thanks to you all,

Fabio M. De Francesco



\
 
 \ /
  Last update: 2021-11-17 08:03    [W:0.128 / U:0.456 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site