linux-kernel - Re: [PATCH] firewire: core: option to log bus reset initiation

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date: Mon, 25 Mar 2024 09:41:34 +0900
From: Takashi Sakamoto <o-takashi@...amocchi.jp>
To: Adam Goldman <adamg@...ox.com>
Cc: linux1394-devel@...ts.sourceforge.net, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] firewire: core: option to log bus reset initiation

Hi Adam,

On Wed, Mar 20, 2024 at 02:14:11AM -0700, Adam Goldman wrote:
> Add a debug parameter to firewire-core, analogous to the one in 
> firewire-ohci. When this is set to 1, log when we schedule, delay, or 
> initiate a bus reset. Since FireWire bus resets can originate from any 
> node on the bus, specific logging of the resets we initiate provides 
> additional insight.
> 
> Signed-off-by: Adam Goldman <adamg@...ox.com>

Thanks for the patch. I applied it to for-next[1].

Now we have two debug parameters per module for the slightly-similar
purpose. In my opinion, it is a pretty cumbersome to enable them when
checking bus-reset behaviour. I think it is time to investigate the other
way.

Linux Kernel Tracepoints[2] is one of options. Roughly describing, the
tracepoints mechanism allows users to deliver structured data from kernel
space to user space via ring-buffer when enabling it by either sysfs or
kernel command-line parameters. Linux kernel also has a command-line
parameter to redirect the human-readable formatted data to kernel log[3].
I think it is suitable in the case.

It requires many work to replace the existent debug parameter of
firewire-ohci, while it is a good start to work just for bus-reset debug.
The data structure layout should be pre-defined in each subsystem, thus we
need to decide it. In my opinion, it would be like:

```
struct bus_reset_event {
    enum reason {
        Initiate,
	Schedule,
	Postpone,
	Detect,
    },
    // We can put any other data if prefering.
}
```

Would I ask your opinion about my idea?

[1] https://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394.git/log/?h=for-next
[2] https://docs.kernel.org/trace/tracepoints.html
[3] 'tp_printk' in kernel/trace/trace.c, 'trace_event' in kernel/trace/trace_event.c

Regards

Takashi Sakamoto