kern: move the vnode user coredumper out into its own file

[kevans]

Summary

This more cleanly contains the bits that are specifically relevant to
dumping coredumps out to a vnode, which will make future changes a bit
easier to review. This also makes the scope of the relevant sysctls
easier to reason about in the process, as they're not visible outside of
the vnode dumper file -- this will mostly become relevant when we allow
pluggable dumpers.

While we're here, move all of the coredump-related stuff out into its
own sys/coredump.h. We have enough that it's useful to separate it out
and de-clutter sys/exec.h a bit.

[kib]

I would also move rest of generic core dump bits into e.g. kern_ucoredump.c (from kern_sig.c).

sys/kern/coredump_vnode.c

30

No need in param.h it is provided by systm.h

59

Use _Static_assert?

62

and below

77

I suggest getting rid of this define, it does not add any value, only obfuscates the real use of the lock.

79

[kevans]

Address review commentary, and split further bits out of kern_sig.c

sys/coredump.h -> sys/ucoredump.h for symmetry with kern_ucoredump.c. The vnode
dumper remains in coredump_vnode.c for the time being to mirror the historical
naming of coredump().

[kevans]

share/man/man5/core.5

119

This could probably use a rewrite in general... the below options correspond directly to values of kern.compress_uesr_cores below being usable, but it's not really tied in that well. I have a draft re-flow of some of the above text prepared for the next review in the stack, because it's a little messy: we talk about the core filename, segue over to signals blocked, then return back to discussing filename configuration. My edit will move the middle bit down to keep all of the naming stuff together, particularly because it's more useful as those two or three paragraphs around and including it are describing the default vnode dumper.

[kib]

sys/kern/kern_sig.c

3636

I would move sigexit() to kern_ucoredump.c as well.

[kevans]

Take sigexit() with us

sig_do_core() is added for the one conditional in sigexit() that wants to check
a sigprop, rather than opening up sigprops more widely -- this is the only
change to the function in the move. Blank line at the beginning of
sig_do_core() is retained for consistency with the rest of the file.

[markj]

sys/kern/kern_sig.c

48

This include of compressor.h can probably go away.

sys/kern/kern_ucoredump.c

214

Extra newline.

[emaste]

share/man/man5/core.5

124–125

Presumably this is a bugfix that needs to be MFC'd (regardless of whether the rest gets MFC'd or not); should it go in separately?

sys/kern/coredump_vnode.c

5

Is this the right copyright statement? A lot of this code was from sef@, alfred@, etc., but kern_sig.c has only Regents & USL.

[kevans]

sys/kern/coredump_vnode.c

5

The original version of this file was mostly copied from kern_exec.c, so I had opted to copy the copyright/license from there... open to ideas on how to label it most accurately, there's a boatload of history to shuffle through.

[kevans]

Tracing the lineage of everything here is challenging, but here are some notes from my basic scouting of some of the more fundamental changes

sys/ucoredump.h:
 - markj: core_write, core_output, coredump_params, compress_user*
   largely imgact_elf.c in aa14e9b7c9d90 / r279801
   moved to exec.h in 33621dfc196e3 by trasz
 - kevans: coredump_writer + friends, coredump_vnode + friends
 - cem: coredump_pack_* (2 lines)

kern_ucoredump.c:
 - kib / emaste did work under the FreeBSD Foundation
   2014 (539c9eef121048 / r272535)
   2017 (6edbe5616c761bf)
 - markj implemented compress_user_cores as it is today
   78f57a9cdef729 / r327707
 - ps implemented kern.coredump
   e5a28db9f5cf27 / r58416 (2000)
 - dg implemented kern.sugid_coredump
   c87e2930e6a89 / r37226 (1998)

coredump_vnode.c:
 - oshogbo rewrote a bunch of logic for %I, resulting in corefile_open_last
   0dea6e3c9834ba / r335844
 - sef implemented kern.corefile / expand_name(), basis for corefile_open
   c5edb423c6271 / r37496 (1998)
 - pjd closed races in vnode open
   c345faea5abdc6 / r244452 (WHEEL systems)
 - alfred implemented initial compressed coredumps
   e7228204343ac / r204552
 - rpaulo implemented the devd notification
   842ab62b051379 / r278479

I think the executive summary is:

• sys/ucoredump.h: BSD-2-Clause, @markj (2015) and kevans (2025)
• kern_ucoredump.c: BSD-3-Clause, Foundation, markj, ps, dg
• coredump_vnode.c: BSD-2-Clause, @rpaulo, @alfred, @pjd, @sef, @oshogbo

I'm quite certain I've missed something somewhere, but this seems like a reasonable start -- I've tagged all of the above in case they want to object or clarify... almsot all of this work was before my involvement in the project. The licenses used are taken from where the majority of the file came from in the most recent iteration, I'm not sure if we need to have a 3-clause vs. 2-clause fight since that's probably not right.

[imp]

In D51349#1175669, @kevans wrote:

Tracing the lineage of everything here is challenging, but here are some notes from my basic scouting of some of the more fundamental changes

sys/ucoredump.h:
 - markj: core_write, core_output, coredump_params, compress_user*
   largely imgact_elf.c in aa14e9b7c9d90 / r279801
   moved to exec.h in 33621dfc196e3 by trasz
 - kevans: coredump_writer + friends, coredump_vnode + friends
 - cem: coredump_pack_* (2 lines)

kern_ucoredump.c:
 - kib / emaste did work under the FreeBSD Foundation
   2014 (539c9eef121048 / r272535)
   2017 (6edbe5616c761bf)
 - markj implemented compress_user_cores as it is today
   78f57a9cdef729 / r327707
 - ps implemented kern.coredump
   e5a28db9f5cf27 / r58416 (2000)
 - dg implemented kern.sugid_coredump
   c87e2930e6a89 / r37226 (1998)

coredump_vnode.c:
 - oshogbo rewrote a bunch of logic for %I, resulting in corefile_open_last
   0dea6e3c9834ba / r335844
 - sef implemented kern.corefile / expand_name(), basis for corefile_open
   c5edb423c6271 / r37496 (1998)
 - pjd closed races in vnode open
   c345faea5abdc6 / r244452 (WHEEL systems)
 - alfred implemented initial compressed coredumps
   e7228204343ac / r204552
 - rpaulo implemented the devd notification
   842ab62b051379 / r278479

I think the executive summary is:

• sys/ucoredump.h: BSD-2-Clause, @markj (2015) and kevans (2025)
• kern_ucoredump.c: BSD-3-Clause, Foundation, markj, ps, dg
• coredump_vnode.c: BSD-2-Clause, @rpaulo, @alfred, @pjd, @sef, @oshogbo

I'm quite certain I've missed something somewhere, but this seems like a reasonable start -- I've tagged all of the above in case they want to object or clarify... almsot all of this work was before my involvement in the project. The licenses used are taken from where the majority of the file came from in the most recent iteration, I'm not sure if we need to have a 3-clause vs. 2-clause fight since that's probably not right.

So I'm a little confused.... Did you move everything to kern_ucoredump.c. If. so: put all three copyrights there and get on with life. The code that I think is moved in this patch (I glanced only, didn't do per-line review).

If this is 'what to put on each file' then one per is fine. Generally copied code gets the copyright of where it was copied from... That's never wrong, but sometimes could be better...

The general rule isn't 'who changes it' so much as 'who contributed a significant amount of creative content' to the file. Those are the people that hold the copyright. If there's been such a significant rewrite of a file since the original author created it. If so, then that person no longer has a copyright to the file (but everybody else committed to it with the original license which implicitly accepts that license and is what their code should be considered to be).

Otherwise, I'm confused...

In any case BSD-3-clause vs BSD-2-clause isn't worth fighting before landing. ps and dg would sign-off on the change, but have high latencies when I've pinged them in the past. If the code they wrote is now de-minimus, then we could change w/o asking and changing the copyright owners. It's nicer to give an ack to the original author "This code was rewritten from code originally by Paul Saab" or similar if you want. I tend to do this when I copy the whole file and use it as a template (say to create a new driver, leaving only boilerplate elements from the original).

Clear as mud, eh?

tl;dr: copying the copyright/license is right, but might be suboptimal. It's necessary and sufficient to commit. However, one might optimize the attribution based on what actually remains, but we have a long history of not bothering except in extreme cases.

sys/kern/coredump_vnode.c

5

So there's three copyrights, per other comments.
Just put all 3 here for now. We have several different places where we do this.

[kevans]

Fix the copyright/license on kern_ucoredump.c

Note which file each license is from, since this is largely code that's simply
moved.

[ziaee]

sys/kern/coredump_vnode.c

2

The hyphen here was for a project Warner abandoned, and it's no longer recommended.

sys/sys/ucoredump.h

1

Same

[kevans]

sys/kern/coredump_vnode.c

2

Ah, weird- I was under the impression this was purely to get indent(1) to avoid touching these blocks. These are copy/paste artifacts, and I don't really feel that strongly about it. Will remove locally.

[imp]

sys/kern/coredump_vnode.c

2

It is.

Most of the files with licenses used this kind of comment for that reason. So that's why i made them all regular. These days, indent is lightly used and clang-format doesn't ovey the convention.. so there's no need for it anymore...