Re: fw_update lock_db should exit when parent exits
On Wed, Aug 23, 2023 at 08:42:03AM -0600, Todd C. Miller wrote: > On Tue, 22 Aug 2023 19:55:56 -0700, Andrew Hewus Fresh wrote: > > > I noticed this when testing how signal handling worked in fw_update, it > > turns out that if you `pkill -KILL -f fw_update` it may leave behind a perl > > process that is locking the package database. Instead of just waiting > > to be killed, we can have that process check to see if its parent is > > still around and exit if not. > > > > Is there a more appropriate solution to this? > > What's the right way to notice your parent exited? > > One way is to have a pipe between the parent and child and use > select() instead of the sleep() to tell when it goes away. I would have to see an example of doing that between ksh and perl.
Re: [patch] netcat: support --crlf
On Wed, 23 Aug 2023, Pietro Cerutti wrote: > Hi, > > here at FreeBSD, we vendor in your netcat with a few local modifications. > > I'm working on adding support to --crlf. I have a diff against the FreeBSD > version here: https://reviews.freebsd.org/D41489 > > I'd like this to be upstreamed. If there's consensus, I'll prepare a patch > against OpenBSD's version and send it over. What is the motivation for this option beyond "Linux has it"? Correct me if I'm wrong but it seems trivial to do this conversion without writing more code by sticking tr in a pipe with nc. OpenBSD's nc doesn't use getopt_long at present and I'm not sure there would be appetite to do it for a single new flag. I note that nc on the Debian machine I have at hand does -C but doesn't recognise --crlf. IMO the long option therefore just adds incompatibility. [djm@dvm ~]$ nc --crlf nc: invalid option -- '-' usage: nc [-46CDdFhklNnrStUuvZz] [-I length] [-i interval] [-M ttl] [-m minttl] [-O length] [-P proxy_username] [-p source_port] [-q seconds] [-s sourceaddr] [-T keyword] [-V rtable] [-W recvlimit] [-w timeout] [-X proxy_protocol] [-x proxy_address[:port]] [destination] [port] [djm@dvm ~]$ uname -a Linux dvm 6.1.0-10-amd64 #1 SMP PREEMPT_DYNAMIC Debian 6.1.38-1 (2023-07-14) x86_64 GNU/Linux -d
dt(4), hardclock(9): move interval, profile providers to dedicated callback
This is the next patch in the clock interrupt reorganization series.
This patch moves the entry points for the interval and profile dt(4)
providers from the hardclock(9) to a dedicated clock interrupt
callback, dt_prov_profile_intr(), in dev/dt/dt_prov_profile.c.
- To preserve current behavior, (1) both provider entrypoints have
been moved into a single callback function, (2) the interrupt runs at
the same frequency as the hardclock, and (3) the interrupt is
staggered to co-occur with the hardclock on a given CPU.
All of this can be changed later. This patch is strictly
logistical: moving these parts out of the hardclock.
- Each dt_pcb gets a provider-specific "dp_clockintr" pointer. If the
PCB's implementing provider needs a clock interrupt to do its work, it
stores the handle in dp_clockintr. The handle, if any, is established
during dtpv_alloc() and disestablished during dtpv_dealloc().
Only the interval and profile providers use it at present.
- In order to implement dt_prov_profile_dealloc() I needed a complete
definition of struct dt_softc, so I hoisted it up out of dt_dev.c
into dtvar.h.
- A PCB's periodic clock interrupt, if any, is started during
dt_ioctl_record_start() stopped during dt_ioctl_record_stop().
This is not a provider-agnostic approach, but I don't see where
else to start/stop the clock interrupt.
One alternative is to start running the clock interrupts when they
are allocated in dtpv_alloc() and stop them when they are freed in
dtpv_dealloc(). This is wasteful, though: the PCBs are not recording
yet, so the interrupts won't perform any useful work until the
controlling process enables recording.
An additional pair of provider hooks, e.g. "dtpv_record_start" and
"dtpv_record_stop", might resolve this.
- We haven't needed to destroy clock interrupts yet, so the
clockintr_disestablish() function used in this patch is new.
The implementation is extremely similar to that of clockintr_cancel().
However, for sake of simplicity, a callback may not disestablish its
clockintr while the callback is running.
One tricky part: if a clockintr is disestablished while it is running,
the dispatch thread takes care not to use-after-free when it re-enters
the clockintr_queue mutex.
I have tested this on amd64. It seems to work: the clock interrupts
fire, stack traces are recorded, and the handles are deallocated when
the process closes the file descriptor.
I will be testing it on other platforms over the next few days.
Thoughts? Test results?
Index: kern/kern_clockintr.c
===
RCS file: /cvs/src/sys/kern/kern_clockintr.c,v
retrieving revision 1.32
diff -u -p -r1.32 kern_clockintr.c
--- kern/kern_clockintr.c 21 Aug 2023 17:22:04 - 1.32
+++ kern/kern_clockintr.c 23 Aug 2023 23:35:11 -
@@ -218,7 +218,7 @@ clockintr_dispatch(void *frame)
{
uint64_t lateness, run = 0, start;
struct cpu_info *ci = curcpu();
- struct clockintr *cl;
+ struct clockintr *cl, *shadow;
struct clockintr_queue *cq = >ci_queue;
u_int ogen;
@@ -257,22 +257,26 @@ clockintr_dispatch(void *frame)
break;
}
clockintr_cancel_locked(cl);
- cq->cq_shadow.cl_expiration = cl->cl_expiration;
+ shadow = >cq_shadow;
+ shadow->cl_expiration = cl->cl_expiration;
+ shadow->cl_func = cl->cl_func;
cq->cq_running = cl;
mtx_leave(>cq_mtx);
- cl->cl_func(>cq_shadow, frame);
+ shadow->cl_func(shadow, frame);
mtx_enter(>cq_mtx);
- cq->cq_running = NULL;
- if (ISSET(cl->cl_flags, CLST_IGNORE_SHADOW)) {
- CLR(cl->cl_flags, CLST_IGNORE_SHADOW);
- CLR(cq->cq_shadow.cl_flags, CLST_SHADOW_PENDING);
- }
- if (ISSET(cq->cq_shadow.cl_flags, CLST_SHADOW_PENDING)) {
- CLR(cq->cq_shadow.cl_flags, CLST_SHADOW_PENDING);
- clockintr_schedule_locked(cl,
- cq->cq_shadow.cl_expiration);
+ if (cq->cq_running != NULL) {
+ cq->cq_running = NULL;
+ if (ISSET(cl->cl_flags, CLST_IGNORE_SHADOW)) {
+ CLR(cl->cl_flags, CLST_IGNORE_SHADOW);
+ CLR(shadow->cl_flags, CLST_SHADOW_PENDING);
+ }
+ if (ISSET(shadow->cl_flags, CLST_SHADOW_PENDING)) {
+ CLR(shadow->cl_flags, CLST_SHADOW_PENDING);
+ clockintr_schedule_locked(cl,
+ shadow->cl_expiration);
+ }
}
run++;
}
@@ -382,6 +386,34 @@
Re: all platforms: separate cpu_initclocks() from cpu_startclock()
On Mon, Aug 21, 2023 at 10:23:53PM -0500, Scott Cheloha wrote: > On Tue, Aug 22, 2023 at 02:36:31AM +, Mike Larkin wrote: > > On Mon, Aug 21, 2023 at 09:26:00PM -0500, Scott Cheloha wrote: > > > On Mon, Aug 21, 2023 at 10:10:58PM +, Mike Larkin wrote: > > > > On Sat, Aug 19, 2023 at 01:44:47PM -0500, Scott Cheloha wrote: > > > > > On Sun, Aug 13, 2023 at 01:48:21PM -0500, Scott Cheloha wrote: > > > > > > This is the next patch in the clock interrupt reorganization series. > > > > > > > > > > > > Before we continue breaking up the hardclock(9) we need to detour > > > > > > into > > > > > > the MD code. > > > > > > > > > > > > This patch divides the "initialization" parts of cpu_initclocks() > > > > > > from > > > > > > the "start the clock interrupt" parts. Seprating the two parts > > > > > > leaves > > > > > > initclocks() an opportunity to prepare the primary CPU for clock > > > > > > interrupt dispatch in a machine-independent manner before actually > > > > > > pulling the trigger. It's nearly impossible to do any MI setup > > > > > > during > > > > > > initclocks() because cpu_initclocks() does everything in one go: > > > > > > both > > > > > > initialization and kickoff are done when cpu_initclocks() returns. > > > > > > > > > > > > Many platforms have a "cpu_startclock()" function, so this patch > > > > > > takes > > > > > > that de facto standard and makes it a rule: cpu_startclock() is now > > > > > > required. It is prototyped in sys/systm.h and every platform must > > > > > > implement it. > > > > > > > > > > > > The revised initclocks() sequence is then: > > > > > > > > > > > > 1. Call cpu_initclocks(). At minimum, cpu_initclocks() ensures > > > > > >hz, stathz, and profhz are initialized. All the machine > > > > > >independent setup in step (2) (currently) depends upon > > > > > >these machine-dependent values. > > > > > > > > > > > > 2. Compute intervals using hz, stathz, and profhz. > > > > > > > > > > > >In a later step I will move the full contents of clockintr_init() > > > > > >up into initclocks() and get rid of clockintr_init() entirely. > > > > > > > > > > > > 3. Call cpu_startclock(). At minimum, cpu_startclock() starts the > > > > > >clock interrupt dispatch cycle on the primary CPU. > > > > > > > > > > > > I have compiled/booted this patch on amd64 (lapic path), arm64, i386 > > > > > > (lapic path), macppc, octeon, and sparc64 (sun4v). > > > > > > > > > > > > I am looking for compile/boot tests on alpha, armv7, hppa, landisk, > > > > > > luna88k, powerpc64, and riscv64. I think armv7 is the tricky one > > > > > > here. Everything else is relatively straightforward, though I may > > > > > > have missed a few stray variables here or there. > > > > > > > > > > > > Test results? Ok? > > > > > > > > > > Here is an updated patch that removes several MD prototypes for > > > > > cpu_startclock() that I missed the first time through. > > > > > > > > > > I went back and tested these again: > > > > > > > > > > - amd64 (lapic) > > > > > - arm64 > > > > > - i386 (lapic) > > > > > - powerpc/macppc > > > > > - mips64/octeon (loongson should be fine) > > > > > - sparc64 (sys_tick; tick/stick should be fine) > > > > > > > > > > arm/armv7 and riscv64 were tested under the previous version, but I > > > > > would appreciate a second compile-test to make sure the header changes > > > > > in the updated patch did not break the build (CC phessler@, jsg@). > > > > > > > > > > I am still seeking compile/boot-tests for the following: > > > > > > > > > > - alpha > > > > > - hppa > > > > > - m88k/luna88k > > > > > > > > if you are really interested in doing this [...] > > > > > > "really interested" is a bit strong. As always, my primary goal is > > > not to break anything when I make a commit. > > > > > > The luna88k patch looks pretty straightfoward, but it's hard to be > > > completely sure I didn't screw something up. > > > > > > > [...] you could run this in nono since you're just looking for > > > > a compile/boot test. > > > > > > Apparently the license forbids redistribution. Super annoying. > > > > so? install it, boot a luna88k "vm", test your diff, then you have your > > question answered. you aren't redistributing anything. > > FWIW, I think vmctl/vmd have a nicer user interface. Same :) > > I feel like I'm... boxing... with nono, not using it.
[patch] netcat: support --crlf
Hi, here at FreeBSD, we vendor in your netcat with a few local modifications. I'm working on adding support to --crlf. I have a diff against the FreeBSD version here: https://reviews.freebsd.org/D41489 I'd like this to be upstreamed. If there's consensus, I'll prepare a patch against OpenBSD's version and send it over. Thanks, -- Pietro Cerutti I have pledged to give 10% of income to effective charities and invite you to join me - https://givingwhatwecan.org
Re: installer: disk crypto: crank KDF rounds to hardware based default
On Fri, Aug 11, 2023 at 03:51:38PM +0100, Stuart Henderson wrote:
> On 2023/08/11 16:43, Mark Kettenis wrote:
> > See the recent discussion about _bcrypt_autorounds() in libc.
> >
> > System performance varies, and even on modern hardware it can provide
> > varying results. The ramdisk environment is considerably different
> > from the mult-user environment in this respect.
> >
> > Using a fixed value is way more predictable. If 16 no longer is a
> > safe default it should be raised.
With this diff and an extra -v, the passphrase through the installer's
question pretty consistently yields 300-350 rounds in my setup, whereas
the same bioctl invocation run in my idle desktop session varies between
ranges from 90 to 350, averaging at 240.
Is that an acceptable raise?
>
> Agreed. (Re bcrypt, I usually completely ignore auto rounds, I had just
> forgotten to set that up on the machine where I noticed the problem..)
>
> Also, am I right in thinking that this only affects the time when
> entering the passphrase when mounting or creating the device, i.e. once
> per boot?
Again, yes.
>
> If so, there's nowhere near as much a downside to that being slow
> as there is for user login. (anyone actually wanting to crack these
> passphrases would be doing it on a fast system rather than whatever
> the device is normally used with, so there are valid reasons for
> picking something that might be a bit slow if it doesn't cause too
> much system impact).
This diff is OK op, who also tested it intensively; regress/sbin/bioctl
will cover '-r' behaviour soon, too.
I agree with Theo here that bioctl's case is different from passwd(1)/libc.
Default [-r auto] never decreases rounds, only explicit '-r N' can.
16 is the absolute minium.
-P to change passphrases picks MAX(old-rounds, auto).
Feedback? Objection? OK?
Index: bioctl.8
===
RCS file: /cvs/src/sbin/bioctl/bioctl.8,v
retrieving revision 1.113
diff -u -p -r1.113 bioctl.8
--- bioctl.821 Aug 2023 08:33:11 - 1.113
+++ bioctl.823 Aug 2023 13:22:38 -
@@ -282,11 +282,12 @@ passphrase into a key, in order to creat
passphrase of an existing encrypted volume.
A larger number of iterations takes more time, but offers increased resistance
against passphrase guessing attacks.
-If
+By default, or if
.Ar rounds
-is specified as "auto", the number of rounds will be automatically determined
-based on system performance.
-Otherwise the minimum is 4 rounds and the default is 16.
+is specified as
+.Cm auto ,
+the number of rounds will automatically be based on system performance.
+The minimum is 16 rounds.
.It Fl s
Read passphrases from
.Pa /dev/stdin
Index: bioctl.c
===
RCS file: /cvs/src/sbin/bioctl/bioctl.c,v
retrieving revision 1.154
diff -u -p -r1.154 bioctl.c
--- bioctl.c21 Aug 2023 08:33:11 - 1.154
+++ bioctl.c23 Aug 2023 13:22:38 -
@@ -89,7 +89,7 @@ int devh = -1;
inthuman;
intverbose;
u_int32_t cflags = 0;
-intrflag = 0;
+intrflag = -1; /* auto */
char *password;
void *bio_cookie;
@@ -182,7 +182,7 @@ main(int argc, char *argv[])
rflag = -1;
break;
}
- rflag = strtonum(optarg, 4, 1<<30, );
+ rflag = strtonum(optarg, 16, 1<<30, );
if (errstr != NULL)
errx(1, "number of KDF rounds is %s: %s",
errstr, optarg);
@@ -979,7 +979,7 @@ bio_kdf_generate(struct sr_crypto_kdfinf
kdfinfo->pbkdf.generic.len = sizeof(kdfinfo->pbkdf);
kdfinfo->pbkdf.generic.type = SR_CRYPTOKDFT_BCRYPT_PBKDF;
- kdfinfo->pbkdf.rounds = rflag ? rflag : 16;
+ kdfinfo->pbkdf.rounds = rflag;
kdfinfo->flags = SR_CRYPTOKDF_KEY | SR_CRYPTOKDF_HINT;
kdfinfo->len = sizeof(*kdfinfo);
@@ -1119,12 +1119,14 @@ bio_changepass(char *dev)
/* Current passphrase. */
bio_kdf_derive(, , "Old passphrase: ", 0);
- /*
-* Unless otherwise specified, keep the previous number of rounds as
-* long as we're using the same KDF.
-*/
- if (kdfhint.generic.type == SR_CRYPTOKDFT_BCRYPT_PBKDF && !rflag)
- rflag = kdfhint.rounds;
+ if (rflag == -1) {
+ rflag = bcrypt_pbkdf_autorounds();
+
+ /* Use previous number of rounds for the same KDF if higher. */
+ if (kdfhint.generic.type == SR_CRYPTOKDFT_BCRYPT_PBKDF &&
+ rflag < kdfhint.rounds)
+ rflag = kdfhint.rounds;
+ }
/* New passphrase. */
bio_kdf_generate();
@@ -1328,7 +1330,7 @@ derive_key(u_int32_t type, int
Re: fw_update lock_db should exit when parent exits
On Tue, 22 Aug 2023 19:55:56 -0700, Andrew Hewus Fresh wrote: > I noticed this when testing how signal handling worked in fw_update, it > turns out that if you `pkill -KILL -f fw_update` it may leave behind a perl > process that is locking the package database. Instead of just waiting > to be killed, we can have that process check to see if its parent is > still around and exit if not. > > Is there a more appropriate solution to this? > What's the right way to notice your parent exited? One way is to have a pipe between the parent and child and use select() instead of the sleep() to tell when it goes away. - todd
iwx background scan fix
When I updated iwx to -77 firmware I made a mistake in setting up
the new version of the scan command, in the particular case of
starting a background scan.
One part of the command says that active scan should be used.
"Active scan" means the device will send probe requests containing a
desired SSID, soliciting immediate probe responses from APs which
announce the desired SSID. This avoids having to wait for the usual
periodically transmitted beacons on every scanned channel across the
supported 2GHz and 5GHz ranges, and thus saves time during the scan.
In another part of the same command we are supposed to tell the
device which SSID to scan for. The bug I spotted is that during a
background scan we do not set any of the bits which select an SSID
from the list of SSIDs the device may scan for (we only ever populate
this list with one entry, thus always set the first bit).
This patch sets the missing bit in the scan command. The device now
seems to be using probe requests during background scans as intended.
With iwx0 in debug mode I am now seeing a shorter list of APs in
/var/log/messages, which always includes APs for the desired SSID
and usually omits any unrelated APs. Roaming between APs seems to work
well in my testing and background scans seem to complete faster than
before.
There is a known fatal firmware error with SYSASSERT 0x20002806 which
sometimes triggers during background scans. I cannot tell whether this
change fixes that issue because I could never trigger the error reliably.
Today it didn't show up at all during my testing even without this fix.
But this particular firmware error is likely related.
Any additional tests? Ok?
diff eb1b93f95d68c8a362dbd50b0d6814511573e654
72a06ec90bd2b911539c1fefdd1ae60a844d884e
commit - eb1b93f95d68c8a362dbd50b0d6814511573e654
commit + 72a06ec90bd2b911539c1fefdd1ae60a844d884e
blob - 26b002045aafdd90cfdfb10d3edd4ea96030ecbc
blob + 7c6e8f9620011e7cf11c41a55fce56af24fccb9d
--- sys/dev/pci/if_iwx.c
+++ sys/dev/pci/if_iwx.c
@@ -425,7 +425,7 @@ voidiwx_scan_umac_fill_ch_p_v6(struct iwx_softc *,
void iwx_scan_umac_fill_general_p_v10(struct iwx_softc *,
struct iwx_scan_general_params_v10 *, uint16_t, int);
void iwx_scan_umac_fill_ch_p_v6(struct iwx_softc *,
- struct iwx_scan_channel_params_v6 *, uint32_t, int, int);
+ struct iwx_scan_channel_params_v6 *, uint32_t, int);
intiwx_umac_scan_v14(struct iwx_softc *, int);
void iwx_mcc_update(struct iwx_softc *, struct iwx_mcc_chub_notif *);
uint8_tiwx_ridx2rate(struct ieee80211_rateset *, int);
@@ -6855,7 +6855,7 @@ iwx_umac_scan_fill_channels(struct iwx_softc *sc,
uint8_t
iwx_umac_scan_fill_channels(struct iwx_softc *sc,
struct iwx_scan_channel_cfg_umac *chan, size_t chan_nitems,
-int n_ssids, int bgscan)
+int n_ssids, uint32_t channel_cfg_flags)
{
struct ieee80211com *ic = >sc_ic;
struct ieee80211_channel *c;
@@ -6886,8 +6886,8 @@ iwx_umac_scan_fill_channels(struct iwx_softc *sc,
chan->v1.iter_count = 1;
chan->v1.iter_interval = htole16(0);
}
- if (n_ssids != 0 && !bgscan)
- chan->flags = htole32(1 << 0); /* select SSID 0 */
+
+ chan->flags = htole32(channel_cfg_flags);
chan++;
nchan++;
}
@@ -7128,12 +7128,12 @@ iwx_scan_umac_fill_ch_p_v6(struct iwx_softc *sc,
void
iwx_scan_umac_fill_ch_p_v6(struct iwx_softc *sc,
struct iwx_scan_channel_params_v6 *cp, uint32_t channel_cfg_flags,
-int n_ssid, int bgscan)
+int n_ssid)
{
cp->flags = IWX_SCAN_CHANNEL_FLAG_ENABLE_CHAN_ORDER;
cp->count = iwx_umac_scan_fill_channels(sc, cp->channel_config,
- nitems(cp->channel_config), n_ssid, bgscan);
+ nitems(cp->channel_config), n_ssid, channel_cfg_flags);
cp->n_aps_override[0] = IWX_SCAN_ADWELL_N_APS_GO_FRIENDLY;
cp->n_aps_override[1] = IWX_SCAN_ADWELL_N_APS_SOCIAL_CHS;
@@ -7188,7 +7188,7 @@ iwx_umac_scan_v14(struct iwx_softc *sc, int bgscan)
}
iwx_scan_umac_fill_ch_p_v6(sc, _p->channel_params, bitmap_ssid,
- n_ssid, bgscan);
+ n_ssid);
hcmd.len[0] = sizeof(*cmd);
hcmd.data[0] = (void *)cmd;
