Re: malloc: handle to be cleaned chunks the same as regular ones
On Sat, Feb 18, 2023 at 04:12:08PM +0100, Otto Moerbeek wrote:
> Hi,
>
> these recent sshd double free issue prompted me to look at malloc
> again. I have something bigger brewing, but this diff makes sure the
> to be cleaned chunks (via freezero() or malloc_conceal) particpate in
> the delayed freeing just as others.
I'd like to see tests/reviews of this.
>
> -Otto
>
> Index: stdlib/malloc.c
> ===
> RCS file: /home/cvs/src/lib/libc/stdlib/malloc.c,v
> retrieving revision 1.276
> diff -u -p -r1.276 malloc.c
> --- stdlib/malloc.c 27 Dec 2022 17:31:09 - 1.276
> +++ stdlib/malloc.c 16 Feb 2023 14:11:50 -
> @@ -1515,42 +1515,38 @@ ofree(struct dir_info **argpool, void *p
> unmap(pool, p, PAGEROUND(sz), clear ? argsz : 0);
> delete(pool, r);
> } else {
> + void *tmp;
> + u_int i;
> +
> /* Validate and optionally canary check */
> struct chunk_info *info = (struct chunk_info *)r->size;
> if (info->size != sz)
> wrterror(pool, "internal struct corrupt");
> find_chunknum(pool, info, p, mopts.chunk_canaries);
> - if (!clear) {
> - void *tmp;
> - int i;
>
> - if (mopts.malloc_freecheck) {
> - for (i = 0; i <= MALLOC_DELAYED_CHUNK_MASK; i++)
> - if (p == pool->delayed_chunks[i])
> - wrterror(pool,
> - "double free %p", p);
> - }
> - junk_free(pool->malloc_junk, p, sz);
> - i = getrbyte(pool) & MALLOC_DELAYED_CHUNK_MASK;
> - tmp = p;
> - p = pool->delayed_chunks[i];
> - if (tmp == p)
> - wrterror(pool, "double free %p", tmp);
> - pool->delayed_chunks[i] = tmp;
> - if (p != NULL) {
> - r = find(pool, p);
> - REALSIZE(sz, r);
> - if (r != NULL)
> - validate_junk(pool, p, sz);
> - }
> - } else if (argsz > 0) {
> - r = find(pool, p);
> - explicit_bzero(p, argsz);
> + if (mopts.malloc_freecheck) {
> + for (i = 0; i <= MALLOC_DELAYED_CHUNK_MASK; i++)
> + if (p == pool->delayed_chunks[i])
> + wrterror(pool,
> + "double free %p", p);
> }
> + if (clear && argsz > 0)
> + explicit_bzero(p, argsz);
> + junk_free(pool->malloc_junk, p, sz);
> +
> + i = getrbyte(pool) & MALLOC_DELAYED_CHUNK_MASK;
> + tmp = p;
> + p = pool->delayed_chunks[i];
> + if (tmp == p)
> + wrterror(pool, "double free %p", p);
> + pool->delayed_chunks[i] = tmp;
> if (p != NULL) {
> + r = find(pool, p);
> if (r == NULL)
> wrterror(pool,
> "bogus pointer (double free?) %p", p);
> + REALSIZE(sz, r);
> + validate_junk(pool, p, sz);
> free_bytes(pool, r, p);
> }
> }
>
Re: Nuke remnants of /dev/io
Thanks Crystal. If somebody wants to commit this, it is OK gnezdo@
Crystal Kolipe writes:
> The iskmemdev function checks for minor number 14 in addition to 0 and 1 on
> the following archs:
>
> amd64, arm64, i386, and riscv64
>
> Device 2, 14 was traditionally /dev/io, which we don't support and so opening
> it will always return ENXIO from mmopen anyway.
>
> We only use iskmemdev in one place in the tree, to return EPERM when trying
> to access /dev/kmem or /dev/mem when securelevel >= 1.
>
> This patch removes the check for minor(dev) == 14 on the four above mentioned
> architectures.
>
> --- sys/arch/amd64/amd64/conf.c.dist Mon Feb 20 18:17:44 2023
> +++ sys/arch/amd64/amd64/conf.c Mon Feb 20 18:29:28 2023
> @@ -313,7 +313,7 @@
> iskmemdev(dev_t dev)
> {
>
> - return (major(dev) == mem_no && (minor(dev) < 2 || minor(dev) == 14));
> + return (major(dev) == mem_no && minor(dev) < 2);
> }
>
> /*
> --- sys/arch/arm64/arm64/conf.c.dist Mon Feb 20 18:18:20 2023
> +++ sys/arch/arm64/arm64/conf.c Mon Feb 20 18:29:14 2023
> @@ -255,7 +255,7 @@
> iskmemdev(dev_t dev)
> {
>
> - return (major(dev) == CMAJ_MM && (minor(dev) < 2 || minor(dev) == 14));
> + return (major(dev) == CMAJ_MM && minor(dev) < 2);
> }
>
> /*
> --- sys/arch/i386/i386/conf.c.distMon Feb 20 18:18:35 2023
> +++ sys/arch/i386/i386/conf.c Mon Feb 20 18:28:51 2023
> @@ -309,7 +309,7 @@
> int
> iskmemdev(dev_t dev)
> {
> - return (major(dev) == mem_no && (minor(dev) < 2 || minor(dev) == 14));
> + return (major(dev) == mem_no && minor(dev) < 2);
> }
>
> /*
> --- sys/arch/riscv64/riscv64/conf.c.dist Mon Feb 20 18:18:48 2023
> +++ sys/arch/riscv64/riscv64/conf.c Mon Feb 20 18:28:35 2023
> @@ -253,7 +253,7 @@
> iskmemdev(dev_t dev)
> {
>
> - return (major(dev) == mem_no && (minor(dev) < 2 || minor(dev) == 14));
> + return (major(dev) == mem_no && minor(dev) < 2);
> }
>
> /*
[patch] Detect and mitigate uncontrolled ACPI GPE storms
Greetings. I am soliciting feedback on a patch to detect and mitigate
uncontrolled ACPI GPE interrupt storms.
Rationale: There have been a number of threads in the recent past on bugs@ and
misc@ with acpi0 spinning a CPU at 100% [1][2][3][4]. The immediate cause is
likely a buggy BIOS and its ACPI implementation. However, this type of bug is
not exclusive to no-name hardware from China, nor is it specific to a
particular hardware vendor, BIOS vendor, or GPE pin. Hardware that is or was
affected can include Intel [5], Lenovo [6], HP [7], ASUS [8], and Apple [9].
I have been testing with a half-dozen ACPI-equipped systems in various states:
storming and behaving, booting and resuming, 7.2 and -current, SMALL and not.
The attached diff uses a minimum 5-second evaluation window, driven by the
firing of ACPI GPE interrupts (no additional accounting thread, etc). An
uncontrolled GPE storm will be logged as such (real number):
Feb 17 22:57:06 acpitest3 /bsd: uncontrolled GPE storm 7242/s, disabling GPE 06
Alternatively, if this is still too close to papering over the problem, perhaps
a smaller diff that only logs the problem, allowing a user see what the storm
is and report it to their BIOS/hardware vendor?
Thank you for your time.
Brian Conway
[1] https://marc.info/?t=16642298181
[2] https://marc.info/?t=16649772664
[3] https://marc.info/?t=16735649053
[4] https://marc.info/?t=16761438961
[5]
https://community.intel.com/t5/Intel-NUCs/APCI-GPE-0x6F-Interrupt-Storm-under-OpenBSD/m-p/1426755
[6]
https://forums.lenovo.com/t5/ThinkPad-T400-T500-and-newer-T/T480s-ACPI-bug/m-p/4057604
[7]
https://h30434.www3.hp.com/t5/Gaming-Notebooks/High-CPU-Usage-System-ACPI-sys-GPE-L6F-Storm-Omen-15-17/td-p/7169255
[8]
https://answers.microsoft.com/en-us/windows/forum/all/stopping-a-gpe-event-acpi-system-interrupts/cec51e6c-1ed4-4369-9e6f-108c4d6333a6
[9] https://bugzilla.kernel.org/show_bug.cgi?id=117481
diff --git sys/dev/acpi/acpi.c sys/dev/acpi/acpi.c
index 853bad1ab..26a5c1702 100644
--- sys/dev/acpi/acpi.c
+++ sys/dev/acpi/acpi.c
@@ -52,6 +52,9 @@
#define APMDEV_NORMAL 0
#define APMDEV_CTL 8
+#define GPE_RATE_MIN_CYCLE 5 /* seconds*/
+#define GPE_RATE_MAX 2000/* per second */
+
#include "wd.h"
#ifdef ACPI_DEBUG
@@ -98,6 +101,8 @@ void acpi_disable_allgpes(struct acpi_softc *);
struct gpe_block *acpi_find_gpe(struct acpi_softc *, int);
void acpi_enable_onegpe(struct acpi_softc *, int);
intacpi_gpe(struct acpi_softc *, int, void *);
+void acpi_init_gpe_rate(struct acpi_softc *, int);
+intacpi_gpe_rate(struct acpi_softc *, int);
void acpi_enable_rungpes(struct acpi_softc *);
@@ -2229,6 +2234,7 @@ acpi_enable_onegpe(struct acpi_softc *sc, int gpe)
dnprintf(50, "enabling GPE %.2x (current: %sabled) %.2x\n",
gpe, (en & mask) ? "en" : "dis", en);
acpi_write_pmreg(sc, ACPIREG_GPE_EN, gpe>>3, en | mask);
+ acpi_init_gpe_rate(sc, gpe);
}
/* Clear all GPEs */
@@ -2307,7 +2313,40 @@ acpi_gpe(struct acpi_softc *sc, int gpe, void *arg)
if (sc->gpe_table[gpe].flags & GPE_LEVEL)
acpi_write_pmreg(sc, ACPIREG_GPE_STS, gpe>>3, mask);
en = acpi_read_pmreg(sc, ACPIREG_GPE_EN, gpe>>3);
- acpi_write_pmreg(sc, ACPIREG_GPE_EN, gpe>>3, en | mask);
+ /* Re-enable if GPE rate passes, otherwise leave disabled */
+ if (!acpi_gpe_rate(sc, gpe))
+ acpi_write_pmreg(sc, ACPIREG_GPE_EN, gpe>>3, en | mask);
+ return (0);
+}
+
+void
+acpi_init_gpe_rate(struct acpi_softc *sc, int gpe)
+{
+ sc->gpe_table[gpe].rate_start = getuptime();
+ sc->gpe_table[gpe].rate_count = 0;
+}
+
+int
+acpi_gpe_rate(struct acpi_softc *sc, int gpe)
+{
+ struct gpe_block *pgpe = >gpe_table[gpe];
+ time_t cycle;
+
+ pgpe->rate_count++;
+ dnprintf(10, "rate GPE %.2x start %lld elapsed %lld count %zu\n", gpe,
+ pgpe->rate_start, getuptime() - pgpe->rate_start, pgpe->rate_count);
+
+ cycle = getuptime() - pgpe->rate_start;
+ if (cycle >= GPE_RATE_MIN_CYCLE) {
+ if (pgpe->rate_count > (GPE_RATE_MAX * cycle)) {
+ printf("uncontrolled GPE storm %lld/s, disabling GPE
%.2x\n",
+ pgpe->rate_count / cycle, gpe);
+ return (1);
+ }
+
+ /* Reset and start a new cycle */
+ acpi_init_gpe_rate(sc, gpe);
+ }
return (0);
}
diff --git sys/dev/acpi/acpivar.h sys/dev/acpi/acpivar.h
index a9b4a2ae9..4e2f47053 100644
--- sys/dev/acpi/acpivar.h
+++ sys/dev/acpi/acpivar.h
@@ -185,6 +185,9 @@ struct gpe_block {
void *arg;
int active;
int flags;
+
+ time_t rate_start;
+ size_t rate_count;
};
struct acpi_devlist {
Re: wsmouse(4): multi-touch buttons again
I do not expect users to find that field, or play around with it if they don't know what they do. It's "hidden" in wsconsctl for a reason. And plainly, I had no time for the wsconsctl part. On 2/23/23 17:25, joshua stein wrote: > On Thu, 23 Feb 2023 at 17:05:53 +0100, Tobias Heider wrote: >> Wow, thank you for looking into this! I've used your version for a few days >> now and it works really well for me (on a m2 macbook air). I actually think >> the default works so well that we can default to 0 for param 143. >> >> IMO we can add it to the tree at this point to give others the change to >> test it before the diff gets even bigger. > > But please add properly named knobs to wsconsctl for this first, and > preferably for all of the other hidden settings that were added with > wstpad. Expecting users to enable, let alone find, > "mouse.tp.param=72:1" is ridiculous. > > On one of my laptops I wanted to disable the middle soft button and > had to dig through the code to figure out that I had to use > "wsconsctl mouse.tp.param=65:0". >
Re: wsmouse(4): multi-touch buttons again
On Thu, Feb 23, 2023 at 10:25:15AM -0600, joshua stein wrote: > On Thu, 23 Feb 2023 at 17:05:53 +0100, Tobias Heider wrote: > > Wow, thank you for looking into this! I've used your version for a few days > > now and it works really well for me (on a m2 macbook air). I actually think > > the default works so well that we can default to 0 for param 143. > > > > IMO we can add it to the tree at this point to give others the change to > > test it before the diff gets even bigger. > > But please add properly named knobs to wsconsctl for this first, and > preferably for all of the other hidden settings that were added with > wstpad. Expecting users to enable, let alone find, > "mouse.tp.param=72:1" is ridiculous. > > On one of my laptops I wanted to disable the middle soft button and > had to dig through the code to figure out that I had to use > "wsconsctl mouse.tp.param=65:0". > Absolutely. I assumed once it goes in it gets a name. I called it tp.mtbuttons in my intitial diff because that was the best thing I could come up with. The libinput docs call it "clickfinger" behavior [1]. I don't have a strong preference as long as it has a name. [1] https://wayland.freedesktop.org/libinput/doc/latest/clickpad-softbuttons.html#clickfinger
Re: wsmouse(4): multi-touch buttons again
On Thu, 23 Feb 2023 at 17:05:53 +0100, Tobias Heider wrote: > Wow, thank you for looking into this! I've used your version for a few days > now and it works really well for me (on a m2 macbook air). I actually think > the default works so well that we can default to 0 for param 143. > > IMO we can add it to the tree at this point to give others the change to > test it before the diff gets even bigger. But please add properly named knobs to wsconsctl for this first, and preferably for all of the other hidden settings that were added with wstpad. Expecting users to enable, let alone find, "mouse.tp.param=72:1" is ridiculous. On one of my laptops I wanted to disable the middle soft button and had to dig through the code to figure out that I had to use "wsconsctl mouse.tp.param=65:0".
Re: wsmouse(4): multi-touch buttons again
On Tue, Feb 21, 2023 at 08:10:36PM +0100, Ulf Brosziewski wrote:
> This diff is an extension of Tobias Heider's proposal, which aims at
> providing "Apple-like" button inputs on clickpads. I have added some
> things in order to approximate the behaviour of other input drivers.
>
> It's a quick shot, and I have no idea whether it is sufficient in
> practice, it certainly needs thorough testing.
>
> The wsconsctl part doesn't provide a named field yet. With a
> recompiled wsconsctl and kernel, the command
>
> # wsconsctl mouse.param=72:1
>
> activates the feature, if it is available (see below).
>
> The patch contains a simple filter for distinguishing the two-finger
> inputs that should trigger right-button events from the ones that
> shouldn't: If the distance between two contacts is small, the driver
> generates a right-button event; if it is greater than some threshold
> value, the second contact will be ignored.
>
> When a touch is resting in the bottom area, it will be ignored, and no
> further filtering applies to the other touches.
>
> You can inspect the threshold value with
>
> # wsconsctl mouse.param=143
>
> and change it with
>
> # wsconsctl mouse.param=143:
>
> The value is given in device units. If the driver for your touchpad is
> imt(4), the default should correspond, roughly, to a distance of 35mm.
> The threshold is reduced by one third if a two-finger click involves a
> touch in the bottom area. (On medium-sized touchpads, this may be
> necessary to leave enough room for left-button clicks performed by the
> thumb while the pointer-controlling touch remains on the touchpad.)
>
> The feature won't work decently on small touchpads, and it cannot work
> on touchpads without MT-support in our kernel. wsmouse checks whether
> a touchpad
> 1) has MT support,
> 2) is a clickpad,
> 3) its resolution is reported to wsmouse,
> 4) it reports a horizontal size greater than 100mm, and
> 5) a vertical size greater than 60mm.
>
> If these conditions aren't met, wsmouse sets the distance limit to -1,
> which blocks the MTBUTTONS feature. I think only imt(4) touchpads can
> meet these criteria; however, the value can be overridden manually or
> programmatically, and ubcmtp and aplms do this on initialization.
> These drivers don't report resolution values; the distance limit will
> be set to a fourth of the length of the touchpad diagonal. That's a
> workaround based on a wild guess, and I couldn't test it with Apple
> hardware. If you want to apply it to an Elantech-v4 touchpad run by
> pms(4), try
>
> # wsconsctl mouse.param=143:0,72:1
>
> (A change from -1 to 0 will trigger the workaround.)
Wow, thank you for looking into this! I've used your version for a few days
now and it works really well for me (on a m2 macbook air). I actually think
the default works so well that we can default to 0 for param 143.
IMO we can add it to the tree at this point to give others the change to
test it before the diff gets even bigger.
>
>
> diff --git a/sbin/wsconsctl/mousecfg.c b/sbin/wsconsctl/mousecfg.c
> index 76a9984bd86..d6609218372 100644
> --- a/sbin/wsconsctl/mousecfg.c
> +++ b/sbin/wsconsctl/mousecfg.c
> @@ -40,9 +40,9 @@
> #define TP_FILTER_FIRST WSMOUSECFG_DX_MAX
> #define TP_FILTER_LAST WSMOUSECFG_SMOOTHING
> #define TP_FEATURES_FIRSTWSMOUSECFG_SOFTBUTTONS
> -#define TP_FEATURES_LAST WSMOUSECFG_DISABLE
> +#define TP_FEATURES_LAST WSMOUSECFG_MTBUTTONS
> #define TP_SETUP_FIRST WSMOUSECFG_LEFT_EDGE
> -#define TP_SETUP_LASTWSMOUSECFG_TAP_THREE_BTNMAP
> +#define TP_SETUP_LASTWSMOUSECFG_MTBTN_MAXDIST
> #define LOG_FIRSTWSMOUSECFG_LOG_INPUT
> #define LOG_LAST WSMOUSECFG_LOG_EVENTS
>
> diff --git a/sys/arch/arm64/dev/aplhidev.c b/sys/arch/arm64/dev/aplhidev.c
> index 265c5196168..b3bf4838fe8 100644
> --- a/sys/arch/arm64/dev/aplhidev.c
> +++ b/sys/arch/arm64/dev/aplhidev.c
> @@ -680,6 +680,10 @@ struct ubcmtp_finger {
> /* Use a constant, synaptics-compatible pressure value for now. */
> #define DEFAULT_PRESSURE 40
>
> +static struct wsmouse_param aplms_wsmousecfg[] = {
> + { WSMOUSECFG_MTBTN_MAXDIST, 0 }, /* 0: Compute a default value. */
> +};
> +
> struct aplms_softc {
> struct device sc_dev;
> struct device *sc_wsmousedev;
> @@ -759,7 +763,8 @@ aplms_configure(struct aplms_softc *sc)
> hw->mt_slots = UBCMTP_MAX_FINGERS;
> hw->flags = WSMOUSEHW_MT_TRACKING;
>
> - return wsmouse_configure(sc->sc_wsmousedev, NULL, 0);
> + return wsmouse_configure(sc->sc_wsmousedev,
> + aplms_wsmousecfg, nitems(aplms_wsmousecfg));
> }
>
> void
> diff --git a/sys/dev/hid/hidmt.c b/sys/dev/hid/hidmt.c
> index 62b500a4f44..9e01fe597bf 100644
> --- a/sys/dev/hid/hidmt.c
> +++ b/sys/dev/hid/hidmt.c
> @@ -103,7 +103,7 @@ hidmt_get_resolution(struct hid_item *h)
> phy_extent *= 10;
>
malloc: change chunk sizes to be multiple of 16 instead of power of 2
Hi,
The basic idea is simple: one of the reasons the recent sshd bug is
potentially exploitable is that a (erroneously) freed malloc chunk
gets re-used in a different role. My malloc has power of two chunk
sizes and so one page of chunks holds many different types of
allocations. Userland malloc has no knowledge of types, we only know
about sizes. So I changed that to use finer-grained chunk sizes.
Originally I thought it would be a *lot* of work, but it's not too
bad: a couple of hours of thinking and a couple of hours coding, which
mostly consisted of hunting for silent assumptions that chunk sizes
are a power of two.
I suspect this is not the final diff, as there is some performance
impact. In particular, sparc64 seems sensitive to these changes. I'm
still investigating why but I wanted to share the current work in
progress anyway.
Yuu can help by testing this.
Thanks,
-Otto
Index: stdlib/malloc.c
===
RCS file: /home/cvs/src/lib/libc/stdlib/malloc.c,v
retrieving revision 1.276
diff -u -p -r1.276 malloc.c
--- stdlib/malloc.c 27 Dec 2022 17:31:09 - 1.276
+++ stdlib/malloc.c 20 Feb 2023 07:33:29 -
@@ -67,6 +67,11 @@
#define MALLOC_CHUNK_LISTS 4
#define CHUNK_CHECK_LENGTH 32
+#define B2SIZE(b) ((b) * MALLOC_MINSIZE)
+#define B2ALLOC(b) ((b) == 0 ? MALLOC_MINSIZE : \
+ (b) * MALLOC_MINSIZE)
+#define BUCKETS(MALLOC_MAXCHUNK / MALLOC_MINSIZE)
+
/*
* We move allocations between half a page and a whole page towards the end,
* subject to alignment constraints. This is the extra headroom we allow.
@@ -144,9 +149,9 @@ struct dir_info {
int mutex;
int malloc_mt; /* multi-threaded mode? */
/* lists of free chunk info structs */
- struct chunk_head chunk_info_list[MALLOC_MAXSHIFT + 1];
+ struct chunk_head chunk_info_list[BUCKETS + 1];
/* lists of chunks with free slots */
- struct chunk_head chunk_dir[MALLOC_MAXSHIFT + 1][MALLOC_CHUNK_LISTS];
+ struct chunk_head chunk_dir[BUCKETS + 1][MALLOC_CHUNK_LISTS];
/* delayed free chunk slots */
void *delayed_chunks[MALLOC_DELAYED_CHUNK_MASK + 1];
u_char rbytes[32]; /* random bytes */
@@ -195,8 +200,7 @@ struct chunk_info {
LIST_ENTRY(chunk_info) entries;
void *page; /* pointer to the page */
u_short canary;
- u_short size; /* size of this page's chunks */
- u_short shift; /* how far to shift for this size */
+ u_short bucket;
u_short free; /* how many free chunks */
u_short total; /* how many chunks */
u_short offset; /* requested size table offset */
@@ -247,11 +251,11 @@ static void malloc_exit(void);
#endif
/* low bits of r->p determine size: 0 means >= page size and r->size holding
- * real size, otherwise low bits are a shift count, or 1 for malloc(0)
+ * real size, otherwise low bits is the bucket + 1
*/
#define REALSIZE(sz, r)\
(sz) = (uintptr_t)(r)->p & MALLOC_PAGEMASK, \
- (sz) = ((sz) == 0 ? (r)->size : ((sz) == 1 ? 0 : (1 << ((sz)-1
+ (sz) = ((sz) == 0 ? (r)->size : B2SIZE((sz) - 1))
static inline void
_MALLOC_LEAVE(struct dir_info *d)
@@ -502,7 +506,7 @@ omalloc_poolinit(struct dir_info *d, int
d->r = NULL;
d->rbytesused = sizeof(d->rbytes);
d->regions_free = d->regions_total = 0;
- for (i = 0; i <= MALLOC_MAXSHIFT; i++) {
+ for (i = 0; i <= BUCKETS; i++) {
LIST_INIT(>chunk_info_list[i]);
for (j = 0; j < MALLOC_CHUNK_LISTS; j++)
LIST_INIT(>chunk_dir[i][j]);
@@ -883,21 +887,13 @@ map(struct dir_info *d, size_t sz, int z
}
static void
-init_chunk_info(struct dir_info *d, struct chunk_info *p, int bits)
+init_chunk_info(struct dir_info *d, struct chunk_info *p, u_int bucket)
{
- int i;
+ u_int i;
- if (bits == 0) {
- p->shift = MALLOC_MINSHIFT;
- p->total = p->free = MALLOC_PAGESIZE >> p->shift;
- p->size = 0;
- p->offset = 0xdead;
- } else {
- p->shift = bits;
- p->total = p->free = MALLOC_PAGESIZE >> p->shift;
- p->size = 1U << bits;
- p->offset = howmany(p->total, MALLOC_BITS);
- }
+ p->bucket = bucket;
+ p->total = p->free = MALLOC_PAGESIZE / B2ALLOC(bucket);
+ p->offset = bucket == 0 ? 0xdead : howmany(p->total, MALLOC_BITS);
p->canary = (u_short)d->canary1;
/* set all valid bits in the bitmap */
@@ -907,18 +903,15 @@
rpki-client: simplify parse_load_crl_from_mft()
Now that the tricky bits are done, here's my suggestion for simplifying
parse_load_crl_from_mft() after claudio's latest commit.
Since we now explicitly want to look in both locations in all cases, it
seems cleanest to drop the loop altogether and to call the function
twice, once for each possible locations.
Index: parser.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/parser.c,v
retrieving revision 1.85
diff -u -p -r1.85 parser.c
--- parser.c23 Feb 2023 09:50:40 - 1.85
+++ parser.c23 Feb 2023 09:53:55 -
@@ -210,46 +210,47 @@ proc_parser_mft_check(const char *fn, st
}
/*
- * Load the correct CRL using the info from the MFT.
+ * Load the CRL from loc using the info from the MFT.
*/
static struct crl *
-parse_load_crl_from_mft(struct entity *entp, struct mft *mft, char **crlfile)
+parse_load_crl_from_mft(struct entity *entp, struct mft *mft, enum location
loc,
+char **crlfile)
{
struct crl *crl = NULL;
unsigned char *f = NULL;
char*fn = NULL;
size_t flen;
- enum locationloc = DIR_TEMP;
- while (1) {
- fn = parse_filepath(entp->repoid, entp->path, mft->crl, loc);
- if (fn == NULL)
- goto next;
+ *crlfile = NULL;
- f = load_file(fn, );
- if (f == NULL && errno != ENOENT)
+ fn = parse_filepath(entp->repoid, entp->path, mft->crl, loc);
+ if (fn == NULL)
+ goto out;
+
+ f = load_file(fn, );
+ if (f == NULL) {
+ if (errno != ENOENT)
warn("parse file %s", fn);
- if (f == NULL)
- goto next;
- if (!valid_hash(f, flen, mft->crlhash, sizeof(mft->crlhash)))
- goto next;
- crl = crl_parse(fn, f, flen);
-
-next:
- free(f);
- f = NULL;
-
- if (crl != NULL) {
- *crlfile = fn;
- return crl;
- }
- free(fn);
- fn = NULL;
- if (loc == DIR_TEMP)
- loc = DIR_VALID;
- else
- return NULL;
+ goto out;
}
+
+ if (!valid_hash(f, flen, mft->crlhash, sizeof(mft->crlhash)))
+ goto out;
+
+ crl = crl_parse(fn, f, flen);
+ if (crl == NULL)
+ goto out;
+
+ *crlfile = fn;
+ free(f);
+
+ return crl;
+
+ out:
+ free(f);
+ free(fn);
+
+ return NULL;
}
/*
@@ -286,7 +287,9 @@ proc_parser_mft_pre(struct entity *entp,
}
free(der);
- *crl = parse_load_crl_from_mft(entp, mft, crlfile);
+ *crl = parse_load_crl_from_mft(entp, mft, DIR_TEMP, crlfile);
+ if (*crl == NULL)
+ *crl = parse_load_crl_from_mft(entp, mft, DIR_VALID, crlfile);
a = valid_ski_aki(*file, , mft->ski, mft->aki);
if (!valid_x509(*file, ctx, x509, a, *crl, errstr)) {
