linux-kernel - Re: [PATCH v3 2/4] devcoredump: Add dev_coredumpm

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-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <50c40ce746f1497cbc36ad82d6d0d3ca3ac28547.camel@sipsolutions.net>
Date: Mon, 25 Mar 2024 18:00:32 +0100
From: Johannes Berg <johannes@...solutions.net>
To: José Roberto de Souza <jose.souza@...el.com>, 
	linux-kernel@...r.kernel.org, intel-xe@...ts.freedesktop.org
Cc: Rodrigo Vivi <rodrigo.vivi@...el.com>, Mukesh Ojha
 <quic_mojha@...cinc.com>,  Jonathan Cavitt <jonathan.cavitt@...el.com>,
 Lucas De Marchi <lucas.demarchi@...el.com>
Subject: Re: [PATCH v3 2/4] devcoredump: Add dev_coredumpm_timeout()

On Mon, 2024-03-04 at 06:39 -0800, José Roberto de Souza wrote:
> Add function to set a custom coredump timeout.
> 
> Current 5-minute timeout may be too short for users to search and
> understand what needs to be done to capture coredump to report bugs.

> + */
> +static inline void dev_coredumpm(struct device *dev, struct module *owner,
> +				 void *data, size_t datalen, gfp_t gfp,
> +				 ssize_t (*read)(char *buffer, loff_t offset, size_t count,
> +						 void *data, size_t datalen),
> +				void (*free)(void *data))

nit: looks like you missed a space on the 'free' line

I don't think we'll actually _solve_ the discussion of whether or not
this makes sense.

I still think it's a bad idea to hang on to the dumps in core kernel
memory since they can be big (I would've said ath12k is big with 55MB,
but Rodrigo said graphics could be up to 2GB!), without being able to
page it out, etc. That's just a waste of memory, for what I don't think
is even a good reason.

So dunno.

However, I also don't like to exercise any power that I might randomly
hold just because I happened to write the code in the first place... And
if you want to shoot yourselves in the foot with any of this, should I
really disagree? I feel I've voiced my objections enough, and Lucas has
also tried to find ways of making a userspace implementation work for
you.

I'd perhaps argue that the documentation for the functions should be
more opinionated and actually recommend against using a large timeout
(like you want) for all these reasons, but other than that, the code
looks fine to me.

johannes