Messages in this thread | | | Date | Mon, 25 May 2020 10:42:18 +0200 | From | Petr Mladek <> | Subject | Re: [PATCH] twist: allow converting pr_devel()/pr_debug() into printk(KERN_DEBUG) |
| |
On Sun 2020-05-24 23:50:34, Tetsuo Handa wrote: > syzbot found a NULL pointer dereference bug inside mptcp_recvmsg() due to > ssock == NULL, but this bug manifested inside selinux_socket_recvmsg() > because pr_debug() was no-op [1]. > > pr_debug("fallback-read subflow=%p", > mptcp_subflow_ctx(ssock->sk)); > copied = sock_recvmsg(ssock, msg, flags);
The NULL pointer deference was found even without this patch. This patch would just cause that it will manifest itself on another place. What is the benefit, please?
> Since console loglevel used by syzkaller will not print KERN_DEBUG > messages to consoles, always evaluating pr_devel()/pr_debug() messages > will not cause too much console output. Thus, let's allow fuzzers to > always evaluate pr_devel()/pr_debug() messages.
I see few drawbacks with this patch:
1. It will cause adding much more messages into the logbuffer even though they are not flushed to the console. It might cause that more important messages will get overridden before they reach console. They might also make hard to read the full log.
2. Crash inside printk() causes recursive messages. They are currently printed into the printk_safe() buffers and there is a bigger risk that they will not reach the console.
3. pr_debug() messages are not printed by default. It is possible that nobody used them for ages. You might get many errors in less maintained code instead in the really used one. I mean that you will get more noise with less gain.
Have you tested this patch by the syzcaller with many runs, please? Did it helped to actually discover more bugs? Did it really made things easier?
I am not able to judge usefulness without more data. My intuition tells me that we should keep the number of syzcaller-related twists as small as possible. Otherwise, syscaller will diverge more and more from reality.
Best Regards, Petr
| |