Re: Send international text with mail(1) - proposal and patches
On Mon, 25 Sep 2023 21:31:08 +0200, Walter wrote:
> Yours are the first technical, functional corrections I got about the
> code. Thanks! Let's go back in time, then. I think that what you're
> telling me can be done by simply replacing "break" for "return" in my
> original function. Tell me what you think, please.
Yesterday I was so tired that I told you nonsense, there's no difference
between puting break or return there, my original funtion already did
what you told me.
--- send.c.orig 2023-09-25 21:01:34.780102611 +0200
+++ send.c 2023-09-25 21:17:11.120117761 +0200
@@ -33,6 +33,10 @@
#include "rcv.h"
#include "extern.h"
++/* To check charset of the message and add the appropiate MIME headers */
++static char nutf8;
++static int not_utf8(FILE *s, int len);
+
static volatile sig_atomic_t sendsignal; /* Interrupted by a signal? */
/*
@@ -341,6 +345,17 @@
else
puts("Null message body; hope that's ok");
}
+
+ /* Check non valid UTF-8 characters in the message */
+ nutf8 = not_utf8(mtf, fsize(mtf));
+ rewind(mtf);
+ if (nutf8 > 1) {
+ savedeadletter(mtf);
+ puts("Invalid or incomplete multibyte or wide character");
+ fputs(". . . message not sent.\n", stderr);
+ exit(1);
+ }
+
/*
* Now, take the user names from the combined
* to and cc lists and do all the alias
@@ -369,7 +384,7 @@
}
if ((cp = value("record")) != NULL)
(void)savemail(expand(cp), mtf);
-
+
/* Setup sendmail arguments. */
*ap++ = "sendmail";
*ap++ = "-i";
@@ -525,6 +540,16 @@
fmt("To:", hp->h_to, fo, w), gotcha++;
if (hp->h_subject != NULL && w & GSUBJECT)
fprintf(fo, "Subject: %s\n", hp->h_subject), gotcha++;
+ if (nutf8 == 0)
+ fprintf(fo, "MIME-Version: 1.0\n"
+ "Content-Type: text/plain; charset=us-ascii\n"
+ "Content-Transfer-Encoding: 7bit\n"),
+ gotcha++;
+ else if (nutf8 == 1)
+ fprintf(fo, "MIME-Version: 1.0\n"
+ "Content-Type: text/plain; charset=utf-8\n"
+ "Content-Transfer-Encoding: 8bit\n"),
+ gotcha++;
if (hp->h_cc != NULL && w & GCC)
fmt("Cc:", hp->h_cc, fo, w), gotcha++;
if (hp->h_bcc != NULL && w & GBCC)
@@ -610,3 +635,59 @@
sendsignal = s;
}
+
+/* Search non valid UTF-8 characters in the message */
+static int
+not_utf8(FILE *fp, int len)
+{
+ int i, n, nonascii;
+ char c;
+ unsigned char s[len];
+
+ i = 0;
+while ((c = getc(fp)) != EOF)
+ s[i++] = c;
+
+ s[i] = '\0';
+
+ i = n = nonascii = 0;
+ while (s[i] != '\0')
+ if (s[i] <= 0x7f) {
+ i++;
+ /* Two bytes case */
+ } else if (s[i] >= 0xc2 && s[i] < 0xe0 &&
+ s[i + 1] >= 0x80 && s[i + 1] <= 0xbf) {
+ i += 2;
+ nonascii++;
+ /* Special three bytes case */
+ } else if ((s[i] == 0xe0 &&
+ s[i + 1] >= 0xa0 && s[i + 1] <= 0xbf &&
+ s[i + 2] >= 0x80 && s[i + 2] <= 0xbf) ||
+ /* Three bytes case */
+ (s[i] > 0xe0 && s[i] < 0xf0 &&
+ s[i + 1] >= 0x80 && s[i + 1] <= 0xbf &&
+ s[i + 2] >= 0x80 && s[i + 2] <= 0xbf)) {
+ i += 3;
+ nonascii++;
+ /* Special four bytes case */
+ } else if ((s[i] == 0xf0 &&
+ s[i + 1] >= 0x90 && s[i + 1] <= 0xbf &&
+ s[i + 2] >= 0x80 && s[i + 2] <= 0xbf &&
+ s[i + 3] >= 0x80 && s[i + 3] <= 0xbf) ||
+ /* Four bytes case */
+ (s[i] > 0xf0 &&
+ s[i + 1] >= 0x80 && s[i + 1] <= 0xbf &&
+ s[i + 2] >= 0x80 && s[i + 2] <= 0xbf &&
+ s[i + 3] >= 0x80 && s[i + 3] <= 0xbf)) {
+ i += 4;
+ nonascii++;
+ } else {
+ n = i + 1;
+ break;
+ }
+
+ if (nonascii)
+ n++;
+
+ return n;
+}
--
Walter
Re: prevent re-upgrade in powerpc64 boot loader
Klemens Nanni wrote: > Are there more reasons or benefits to this approach than a) less intrusive > chmod() and b) more shared, overall less code? > > I obviously don't see the full picture (yet). A normal bootloader only reads from disk. No write occur. The chmod hack is a write. In the libsa case, it narrowly modifies just the block that is required, because libsa's filesystem code is not a buffering filesystem implimentation. But these octeon / powerpc64 bootloaders are real kernels, running the real ffs. When the "allow a block write" operation is activated, the root filesystem is *REALLY MOUNTED*, which means all the ffs read, read-ahead, inode update, and other write operations come into play. There is no complete "only do what libsa does" scheme. So the worry is that doing mount rw, chmod, mount ro, can create (or make worse) existing filesystem damage, to the point where maybe you cannot boot at all anymore.
Re: Send international text with mail(1) - proposal and patches
On Mon, 25 Sep 2023 19:00:15 +0200, Hiltjo Posthuma wrote:
> On Mon, Sep 25, 2023 at 03:13:03PM +0200, Walter Alejandro Iglesias wrote:
> > This new version, when it detects invalid utf-8 in the body saves a
> > dead.letter, prints the following message and exits.
> >
> > $ mail -s hello user < invalid_utf8.txt
> > Invalid or incomplete multibyte or wide character
> > . . . message not sent.
> >
> >
> >
> > Index: send.c
> > ===
> > RCS file: /cvs/src/usr.bin/mail/send.c,v
> > retrieving revision 1.26
> > diff -u -p -r1.26 send.c
> > --- send.c 8 Mar 2023 04:43:11 - 1.26
> > +++ send.c 25 Sep 2023 13:07:17 -
> > @@ -32,6 +32,11 @@
> >
> > #include "rcv.h"
> > #include "extern.h"
> > +#include "locale.h"
> > +
> > +/* To check charset of the message and add the appropiate MIME headers */
> > +static char nutf8;
> > +static int not_utf8(FILE *s, int len);
> >
> > static volatile sig_atomic_t sendsignal; /* Interrupted by a signal? */
> >
> > @@ -341,6 +346,17 @@ mail1(struct header *hp, int printheader
> > else
> > puts("Null message body; hope that's ok");
> > }
> > +
> > + /* Check non valid UTF-8 characters in the message */
> > + nutf8 = not_utf8(mtf, fsize(mtf));
> > + rewind(mtf);
> > + if (nutf8 > 1) {
> > + savedeadletter(mtf);
> > + puts("Invalid or incomplete multibyte or wide character");
> > + fputs(". . . message not sent.\n", stderr);
> > + exit(1);
> > + }
> > +
> > /*
> > * Now, take the user names from the combined
> > * to and cc lists and do all the alias
> > @@ -520,15 +536,30 @@ puthead(struct header *hp, FILE *fo, int
> > gotcha = 0;
> > from = hp->h_from ? hp->h_from : value("from");
> > if (from != NULL)
> > - fprintf(fo, "From: %s\n", from), gotcha++;
> > + fprintf(fo, "From: %s\n", from),
> > + gotcha++;
> > if (hp->h_to != NULL && w & GTO)
> > - fmt("To:", hp->h_to, fo, w), gotcha++;
> > + fmt("To:", hp->h_to, fo, w),
> > + gotcha++;
> > if (hp->h_subject != NULL && w & GSUBJECT)
> > - fprintf(fo, "Subject: %s\n", hp->h_subject), gotcha++;
> > + fprintf(fo, "Subject: %s\n", hp->h_subject),
> > + gotcha++;
> > + if (nutf8 == 0)
> > + fprintf(fo, "MIME-Version: 1.0\n"
> > + "Content-Type: text/plain; charset=us-ascii\n"
> > + "Content-Transfer-Encoding: 7bit\n"),
> > + gotcha++;
> > + else if (nutf8 == 1)
> > + fprintf(fo, "MIME-Version: 1.0\n"
> > + "Content-Type: text/plain; charset=utf-8\n"
> > + "Content-Transfer-Encoding: 8bit\n"),
> > + gotcha++;
> > if (hp->h_cc != NULL && w & GCC)
> > - fmt("Cc:", hp->h_cc, fo, w), gotcha++;
> > + fmt("Cc:", hp->h_cc, fo, w),
> > + gotcha++;
> > if (hp->h_bcc != NULL && w & GBCC)
> > - fmt("Bcc:", hp->h_bcc, fo, w), gotcha++;
> > + fmt("Bcc:", hp->h_bcc, fo, w),
> > + gotcha++;
> > if (gotcha && w & GNL)
> > (void)putc('\n', fo);
> > return(0);
> > @@ -609,4 +640,44 @@ sendint(int s)
> > {
> >
> > sendsignal = s;
> > +}
> > +
> > +/* Search non valid UTF-8 characters in the message */
> > +static int
> > +not_utf8(FILE *message, int len)
> > +{
>
> Nitpick: I would call `message` maybe `fp` or something here.
>
> > + int c, count, n, ulen;
> > + size_t i, resize;
> > + size_t jump = 100;
> > + unsigned char *s = NULL;
> > +
> > + setlocale(LC_CTYPE, "en_US.UTF-8");
> > +
>
> Should setlocale() be restored later on?
>
> > + if (s == NULL && (s = malloc(jump)) == NULL)
> > + err(1, NULL);
>
> The check if `s` is NULL seems unncessary here.
>
> > +
> > + i = count = 0;
> > + while ((c = getc(message)) != EOF) {
> > + if (s == NULL || count == jump) {
>
> The check if `s` is NULL seems unncessary here.
>
> > + if ((s = realloc(s, i + jump + 1)) == NULL)
> > + err(1, NULL);
> > + count = 0;
> > + }
> > + s[i++] = c;
> > + count++;
> > + }
> > +
> > + s[i] = '\0';
> > +
> > + ulen = mbstowcs(NULL, s, 0);
> > +
> > + if (ulen == len)
> > + n = 0;
> > + else if (ulen < 0)
> > + n = 2;
> > + else if (ulen < len)
> > + n = 1;
> > +
> > + free(s);
> > + return n;
> > }
> >
> >
> > --
> > Walter
> >
>
> Since it assumes UTF-8, maybe mbstowcs() is not needed and it can be done in
> one pass while reading the stream (no need to allocate, slurp the whole file
> and decode). Just: read the per byte and return on the first invalid sequence.
Yours are the first technical, functional corrections I got about the
code. Thanks! Let's go back in time, then.
Re: vmd(8): fix deadlock during pausing
On Sun, Sep 24, 2023 at 01:07:43AM -0400, Dave Voutila wrote:
> vmd has a sneaky little deadlock hidden in the pause logic related to
> the use of mutexes and condition variables.
>
> When pausing, the vcpu is holding the "run" mutex. It then sleeps
> waiting for the unpause condition. If the event thread is trying to
> assert an irq, it will try to lock that "run" mutex in an attempt to
> signal a halted vcpu that it should start running. This deadlocks the
> thread.
>
> Diff below releases the run mutex (by the vcpu thread) before sleeping
> on the "pause" condition and reacquires it afterwards. This lets the
> event thread advance as needed.
>
> A simple reproducer is run `iperf3 -s` inside a guest, `iperf -c -t
> 60` from outside the guest, and them `vmctl pause `. vmctl will hang
> as the vm can't respond to the pause request.
>
> I'm also simplifying the offending vcpu_assert_pic_irq (called by the
> event thread), so the diff should be pretty self contained for review.
>
> ok?
ok mlarkin
>
> diffstat /usr/src
> M usr.sbin/vmd/vm.c | 6+ 8-
>
> 1 file changed, 6 insertions(+), 8 deletions(-)
>
> diff /usr/src
> commit - 89fcd96b33617c4eddd2306889179a96a934ebe8
> path + /usr/src
> blob - fe804b4e3b9e8b59b883e58f02e15a178c035742
> file + usr.sbin/vmd/vm.c
> --- usr.sbin/vmd/vm.c
> +++ usr.sbin/vmd/vm.c
> @@ -1564,16 +1564,20 @@ vcpu_run_loop(void *arg)
> __func__, (int)ret);
> return ((void *)ret);
> }
>
> + /* i8259 may be firing as we pause, release run mtx. */
> + mutex_unlock(_run_mtx[n]);
> ret = pthread_cond_wait(_unpause_cond[n],
> _unpause_mtx[n]);
> if (ret) {
> log_warnx(
> "%s: can't wait on unpause cond (%d)",
> __func__, (int)ret);
> break;
> }
> + mutex_lock(_run_mtx[n]);
> +
> ret = pthread_mutex_unlock(_unpause_mtx[n]);
> if (ret) {
> log_warnx("%s: can't unlock unpause mtx (%d)",
> __func__, (int)ret);
> @@ -2135,20 +2139,14 @@ vcpu_assert_pic_irq(uint32_t vm_id, uint32_t vcpu_id,
>
> if (i8259_is_pending()) {
> if (vcpu_pic_intr(vm_id, vcpu_id, 1))
> fatalx("%s: can't assert INTR", __func__);
> -
> - ret = pthread_mutex_lock(_run_mtx[vcpu_id]);
> - if (ret)
> - fatalx("%s: can't lock vcpu mtx (%d)", __func__, ret);
> -
> + mutex_lock(_run_mtx[vcpu_id]);
> vcpu_hlt[vcpu_id] = 0;
> ret = pthread_cond_signal(_run_cond[vcpu_id]);
> if (ret)
> fatalx("%s: can't signal (%d)", __func__, ret);
> - ret = pthread_mutex_unlock(_run_mtx[vcpu_id]);
> - if (ret)
> - fatalx("%s: can't unlock vcpu mtx (%d)", __func__, ret);
> + mutex_unlock(_run_mtx[vcpu_id]);
> }
> }
>
> /*
>
Re: Send international text with mail(1) - proposal and patches
On Mon, Sep 25, 2023 at 03:13:03PM +0200, Walter Alejandro Iglesias wrote:
> This new version, when it detects invalid utf-8 in the body saves a
> dead.letter, prints the following message and exits.
>
> $ mail -s hello user < invalid_utf8.txt
> Invalid or incomplete multibyte or wide character
> . . . message not sent.
>
>
>
> Index: send.c
> ===
> RCS file: /cvs/src/usr.bin/mail/send.c,v
> retrieving revision 1.26
> diff -u -p -r1.26 send.c
> --- send.c8 Mar 2023 04:43:11 - 1.26
> +++ send.c25 Sep 2023 13:07:17 -
> @@ -32,6 +32,11 @@
>
> #include "rcv.h"
> #include "extern.h"
> +#include "locale.h"
> +
> +/* To check charset of the message and add the appropiate MIME headers */
> +static char nutf8;
> +static int not_utf8(FILE *s, int len);
>
> static volatile sig_atomic_t sendsignal; /* Interrupted by a signal? */
>
> @@ -341,6 +346,17 @@ mail1(struct header *hp, int printheader
> else
> puts("Null message body; hope that's ok");
> }
> +
> + /* Check non valid UTF-8 characters in the message */
> + nutf8 = not_utf8(mtf, fsize(mtf));
> + rewind(mtf);
> + if (nutf8 > 1) {
> + savedeadletter(mtf);
> + puts("Invalid or incomplete multibyte or wide character");
> + fputs(". . . message not sent.\n", stderr);
> + exit(1);
> + }
> +
> /*
>* Now, take the user names from the combined
>* to and cc lists and do all the alias
> @@ -520,15 +536,30 @@ puthead(struct header *hp, FILE *fo, int
> gotcha = 0;
> from = hp->h_from ? hp->h_from : value("from");
> if (from != NULL)
> - fprintf(fo, "From: %s\n", from), gotcha++;
> + fprintf(fo, "From: %s\n", from),
> + gotcha++;
> if (hp->h_to != NULL && w & GTO)
> - fmt("To:", hp->h_to, fo, w), gotcha++;
> + fmt("To:", hp->h_to, fo, w),
> + gotcha++;
> if (hp->h_subject != NULL && w & GSUBJECT)
> - fprintf(fo, "Subject: %s\n", hp->h_subject), gotcha++;
> + fprintf(fo, "Subject: %s\n", hp->h_subject),
> + gotcha++;
> + if (nutf8 == 0)
> + fprintf(fo, "MIME-Version: 1.0\n"
> + "Content-Type: text/plain; charset=us-ascii\n"
> + "Content-Transfer-Encoding: 7bit\n"),
> + gotcha++;
> + else if (nutf8 == 1)
> + fprintf(fo, "MIME-Version: 1.0\n"
> + "Content-Type: text/plain; charset=utf-8\n"
> + "Content-Transfer-Encoding: 8bit\n"),
> + gotcha++;
> if (hp->h_cc != NULL && w & GCC)
> - fmt("Cc:", hp->h_cc, fo, w), gotcha++;
> + fmt("Cc:", hp->h_cc, fo, w),
> + gotcha++;
> if (hp->h_bcc != NULL && w & GBCC)
> - fmt("Bcc:", hp->h_bcc, fo, w), gotcha++;
> + fmt("Bcc:", hp->h_bcc, fo, w),
> + gotcha++;
> if (gotcha && w & GNL)
> (void)putc('\n', fo);
> return(0);
> @@ -609,4 +640,44 @@ sendint(int s)
> {
>
> sendsignal = s;
> +}
> +
> +/* Search non valid UTF-8 characters in the message */
> +static int
> +not_utf8(FILE *message, int len)
> +{
Nitpick: I would call `message` maybe `fp` or something here.
> + int c, count, n, ulen;
> + size_t i, resize;
> + size_t jump = 100;
> + unsigned char *s = NULL;
> +
> + setlocale(LC_CTYPE, "en_US.UTF-8");
> +
Should setlocale() be restored later on?
> + if (s == NULL && (s = malloc(jump)) == NULL)
> + err(1, NULL);
The check if `s` is NULL seems unncessary here.
> +
> + i = count = 0;
> + while ((c = getc(message)) != EOF) {
> + if (s == NULL || count == jump) {
The check if `s` is NULL seems unncessary here.
> + if ((s = realloc(s, i + jump + 1)) == NULL)
> + err(1, NULL);
> + count = 0;
> + }
> + s[i++] = c;
> + count++;
> + }
> +
> + s[i] = '\0';
> +
> + ulen = mbstowcs(NULL, s, 0);
> +
> + if (ulen == len)
> + n = 0;
> + else if (ulen < 0)
> + n = 2;
> + else if (ulen < len)
> + n = 1;
> +
> + free(s);
> + return n;
> }
>
>
> --
> Walter
>
Since it assumes UTF-8, maybe mbstowcs() is not needed and it can be done in
one pass while reading the stream (no need to allocate, slurp the whole file
and decode). Just: read the per byte and return on the first invalid sequence.
--
Kind regards,
Hiltjo
Re: rpki-client: refactor sbgp_assysnum() and sbgp_ipaddrblk()
On Mon, Sep 25, 2023 at 05:02:06PM +0200, Theo Buehler wrote:
> On Mon, Sep 25, 2023 at 04:43:31PM +0200, Claudio Jeker wrote:
> > On Mon, Sep 25, 2023 at 04:38:48PM +0200, Theo Buehler wrote:
> > > On Mon, Sep 25, 2023 at 02:47:37PM +0200, Claudio Jeker wrote:
> > > > On Sat, Sep 23, 2023 at 01:23:34PM +0200, Theo Buehler wrote:
> > > > > This is a second chunk split out of the diff mentioned in my previous
> > > > > mail. It factors the parsing of ASIdentifiers and IPAddrBlocks out of
> > > > > sbgp_assysnum() and sbgp_ipaddrblk() and makes the latter only extract
> > > > > the info from the X509_EXTENSION. This should not change anything, but
> > > > > the logic is a bit tricky.
> > > > >
> > > > > We could initialize *as and *asz, as well as *ips and *ipsz to NULL/0,
> > > > > at the top of the two new sbgp_parse_*.
> > > >
> > > > It looks inded like nthing is changed. The thing I dislike a bit is how
> > > > **as and *asz are updated inside the sbgp_parse_* functions. There is
> > > > return 0 before and after the calloc / recallocarray calls and so it
> > > > depends a lot on the caller to be careful here. The code right now is
> > > > ok.
> > >
> > > Thanks for that clue. I didn't particularly like my diff either. The
> > > below is better, has less churn and should be easier to review. This way
> > > the caller doesn't have to be careful.
> > >
> > > I left the currently existing variables asz and ipsz untouched since it
> > > becomes too confusing. I want to rename asz -> sz and new_asz -> asz in
> > > a follow-up, similarly for ipsz.
> >
> > Indeed much better. OK claudio@
>
> And here's the rename. It is mechanical apart from two lines where I
> fixed the order of the variable declarations and from a line I
> unwrapped.
OK claudio@
> Index: cert.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
> retrieving revision 1.116
> diff -u -p -r1.116 cert.c
> --- cert.c25 Sep 2023 14:56:20 - 1.116
> +++ cert.c25 Sep 2023 14:58:57 -
> @@ -159,7 +159,7 @@ sbgp_parse_assysnum(const char *fn, cons
> {
> const ASIdOrRanges *aors = NULL;
> struct cert_as *as = NULL;
> - size_t asz, new_asz = 0;
> + size_t asz = 0, sz;
> int i;
>
> assert(*out_as == NULL && *out_asz == 0);
> @@ -178,11 +178,11 @@ sbgp_parse_assysnum(const char *fn, cons
>
> switch (asidentifiers->asnum->type) {
> case ASIdentifierChoice_inherit:
> - asz = 1;
> + sz = 1;
> break;
> case ASIdentifierChoice_asIdsOrRanges:
> aors = asidentifiers->asnum->u.asIdsOrRanges;
> - asz = sk_ASIdOrRange_num(aors);
> + sz = sk_ASIdOrRange_num(aors);
> break;
> default:
> warnx("%s: RFC 3779 section 3.2.3.2: ASIdentifierChoice: "
> @@ -190,21 +190,21 @@ sbgp_parse_assysnum(const char *fn, cons
> goto out;
> }
>
> - if (asz == 0) {
> + if (sz == 0) {
> warnx("%s: RFC 6487 section 4.8.11: empty asIdsOrRanges", fn);
> goto out;
> }
> - if (asz >= MAX_AS_SIZE) {
> + if (sz >= MAX_AS_SIZE) {
> warnx("%s: too many AS number entries: limit %d",
> fn, MAX_AS_SIZE);
> goto out;
> }
> - as = calloc(asz, sizeof(struct cert_as));
> + as = calloc(sz, sizeof(struct cert_as));
> if (as == NULL)
> err(1, NULL);
>
> if (aors == NULL) {
> - if (!sbgp_as_inherit(fn, as, _asz))
> + if (!sbgp_as_inherit(fn, as, ))
> goto out;
> }
>
> @@ -214,11 +214,11 @@ sbgp_parse_assysnum(const char *fn, cons
> aor = sk_ASIdOrRange_value(aors, i);
> switch (aor->type) {
> case ASIdOrRange_id:
> - if (!sbgp_as_id(fn, as, _asz, aor->u.id))
> + if (!sbgp_as_id(fn, as, , aor->u.id))
> goto out;
> break;
> case ASIdOrRange_range:
> - if (!sbgp_as_range(fn, as, _asz, aor->u.range))
> + if (!sbgp_as_range(fn, as, , aor->u.range))
> goto out;
> break;
> default:
> @@ -229,7 +229,7 @@ sbgp_parse_assysnum(const char *fn, cons
> }
>
> *out_as = as;
> - *out_asz = new_asz;
> + *out_asz = asz;
>
> return 1;
>
> @@ -361,7 +361,7 @@ sbgp_parse_ipaddrblk(const char *fn, con
> const IPAddressOrRange *aor;
> enum afi afi;
> struct cert_ip *ips = NULL;
> - size_t ipsz, new_ipsz = 0;
> + size_t ipsz = 0, sz;
> int i, j;
>
> assert(*out_ips == NULL && *out_ipsz ==
Re: rpki-client: refactor sbgp_assysnum() and sbgp_ipaddrblk()
On Mon, Sep 25, 2023 at 04:43:31PM +0200, Claudio Jeker wrote:
> On Mon, Sep 25, 2023 at 04:38:48PM +0200, Theo Buehler wrote:
> > On Mon, Sep 25, 2023 at 02:47:37PM +0200, Claudio Jeker wrote:
> > > On Sat, Sep 23, 2023 at 01:23:34PM +0200, Theo Buehler wrote:
> > > > This is a second chunk split out of the diff mentioned in my previous
> > > > mail. It factors the parsing of ASIdentifiers and IPAddrBlocks out of
> > > > sbgp_assysnum() and sbgp_ipaddrblk() and makes the latter only extract
> > > > the info from the X509_EXTENSION. This should not change anything, but
> > > > the logic is a bit tricky.
> > > >
> > > > We could initialize *as and *asz, as well as *ips and *ipsz to NULL/0,
> > > > at the top of the two new sbgp_parse_*.
> > >
> > > It looks inded like nthing is changed. The thing I dislike a bit is how
> > > **as and *asz are updated inside the sbgp_parse_* functions. There is
> > > return 0 before and after the calloc / recallocarray calls and so it
> > > depends a lot on the caller to be careful here. The code right now is ok.
> >
> > Thanks for that clue. I didn't particularly like my diff either. The
> > below is better, has less churn and should be easier to review. This way
> > the caller doesn't have to be careful.
> >
> > I left the currently existing variables asz and ipsz untouched since it
> > becomes too confusing. I want to rename asz -> sz and new_asz -> asz in
> > a follow-up, similarly for ipsz.
>
> Indeed much better. OK claudio@
And here's the rename. It is mechanical apart from two lines where I
fixed the order of the variable declarations and from a line I
unwrapped.
Index: cert.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
retrieving revision 1.116
diff -u -p -r1.116 cert.c
--- cert.c 25 Sep 2023 14:56:20 - 1.116
+++ cert.c 25 Sep 2023 14:58:57 -
@@ -159,7 +159,7 @@ sbgp_parse_assysnum(const char *fn, cons
{
const ASIdOrRanges *aors = NULL;
struct cert_as *as = NULL;
- size_t asz, new_asz = 0;
+ size_t asz = 0, sz;
int i;
assert(*out_as == NULL && *out_asz == 0);
@@ -178,11 +178,11 @@ sbgp_parse_assysnum(const char *fn, cons
switch (asidentifiers->asnum->type) {
case ASIdentifierChoice_inherit:
- asz = 1;
+ sz = 1;
break;
case ASIdentifierChoice_asIdsOrRanges:
aors = asidentifiers->asnum->u.asIdsOrRanges;
- asz = sk_ASIdOrRange_num(aors);
+ sz = sk_ASIdOrRange_num(aors);
break;
default:
warnx("%s: RFC 3779 section 3.2.3.2: ASIdentifierChoice: "
@@ -190,21 +190,21 @@ sbgp_parse_assysnum(const char *fn, cons
goto out;
}
- if (asz == 0) {
+ if (sz == 0) {
warnx("%s: RFC 6487 section 4.8.11: empty asIdsOrRanges", fn);
goto out;
}
- if (asz >= MAX_AS_SIZE) {
+ if (sz >= MAX_AS_SIZE) {
warnx("%s: too many AS number entries: limit %d",
fn, MAX_AS_SIZE);
goto out;
}
- as = calloc(asz, sizeof(struct cert_as));
+ as = calloc(sz, sizeof(struct cert_as));
if (as == NULL)
err(1, NULL);
if (aors == NULL) {
- if (!sbgp_as_inherit(fn, as, _asz))
+ if (!sbgp_as_inherit(fn, as, ))
goto out;
}
@@ -214,11 +214,11 @@ sbgp_parse_assysnum(const char *fn, cons
aor = sk_ASIdOrRange_value(aors, i);
switch (aor->type) {
case ASIdOrRange_id:
- if (!sbgp_as_id(fn, as, _asz, aor->u.id))
+ if (!sbgp_as_id(fn, as, , aor->u.id))
goto out;
break;
case ASIdOrRange_range:
- if (!sbgp_as_range(fn, as, _asz, aor->u.range))
+ if (!sbgp_as_range(fn, as, , aor->u.range))
goto out;
break;
default:
@@ -229,7 +229,7 @@ sbgp_parse_assysnum(const char *fn, cons
}
*out_as = as;
- *out_asz = new_asz;
+ *out_asz = asz;
return 1;
@@ -361,7 +361,7 @@ sbgp_parse_ipaddrblk(const char *fn, con
const IPAddressOrRange *aor;
enum afi afi;
struct cert_ip *ips = NULL;
- size_t ipsz, new_ipsz = 0;
+ size_t ipsz = 0, sz;
int i, j;
assert(*out_ips == NULL && *out_ipsz == 0);
@@ -372,27 +372,26 @@ sbgp_parse_ipaddrblk(const char *fn, con
switch (af->ipAddressChoice->type) {
case IPAddressChoice_inherit:
Re: rpki-client: refactor sbgp_assysnum() and sbgp_ipaddrblk()
On Mon, Sep 25, 2023 at 04:38:48PM +0200, Theo Buehler wrote:
> On Mon, Sep 25, 2023 at 02:47:37PM +0200, Claudio Jeker wrote:
> > On Sat, Sep 23, 2023 at 01:23:34PM +0200, Theo Buehler wrote:
> > > This is a second chunk split out of the diff mentioned in my previous
> > > mail. It factors the parsing of ASIdentifiers and IPAddrBlocks out of
> > > sbgp_assysnum() and sbgp_ipaddrblk() and makes the latter only extract
> > > the info from the X509_EXTENSION. This should not change anything, but
> > > the logic is a bit tricky.
> > >
> > > We could initialize *as and *asz, as well as *ips and *ipsz to NULL/0,
> > > at the top of the two new sbgp_parse_*.
> >
> > It looks inded like nthing is changed. The thing I dislike a bit is how
> > **as and *asz are updated inside the sbgp_parse_* functions. There is
> > return 0 before and after the calloc / recallocarray calls and so it
> > depends a lot on the caller to be careful here. The code right now is ok.
>
> Thanks for that clue. I didn't particularly like my diff either. The
> below is better, has less churn and should be easier to review. This way
> the caller doesn't have to be careful.
>
> I left the currently existing variables asz and ipsz untouched since it
> becomes too confusing. I want to rename asz -> sz and new_asz -> asz in
> a follow-up, similarly for ipsz.
Indeed much better. OK claudio@
> Index: cert.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
> retrieving revision 1.115
> diff -u -p -r1.115 cert.c
> --- cert.c12 Sep 2023 09:33:30 - 1.115
> +++ cert.c25 Sep 2023 14:29:56 -
> @@ -153,40 +153,26 @@ sbgp_as_inherit(const char *fn, struct c
> return append_as(fn, ases, asz, );
> }
>
> -/*
> - * Parse RFC 6487 4.8.11 X509v3 extension, with syntax documented in RFC
> - * 3779 starting in section 3.2.
> - * Returns zero on failure, non-zero on success.
> - */
> -static int
> -sbgp_assysnum(struct parse *p, X509_EXTENSION *ext)
> +int
> +sbgp_parse_assysnum(const char *fn, const ASIdentifiers *asidentifiers,
> +struct cert_as **out_as, size_t *out_asz)
> {
> - ASIdentifiers *asidentifiers = NULL;
> const ASIdOrRanges *aors = NULL;
> - size_t asz;
> - int i, rc = 0;
> + struct cert_as *as = NULL;
> + size_t asz, new_asz = 0;
> + int i;
>
> - if (!X509_EXTENSION_get_critical(ext)) {
> - warnx("%s: RFC 6487 section 4.8.11: autonomousSysNum: "
> - "extension not critical", p->fn);
> - goto out;
> - }
> -
> - if ((asidentifiers = X509V3_EXT_d2i(ext)) == NULL) {
> - warnx("%s: RFC 6487 section 4.8.11: autonomousSysNum: "
> - "failed extension parse", p->fn);
> - goto out;
> - }
> + assert(*out_as == NULL && *out_asz == 0);
>
> if (asidentifiers->rdi != NULL) {
> warnx("%s: RFC 6487 section 4.8.11: autonomousSysNum: "
> - "should not have RDI values", p->fn);
> + "should not have RDI values", fn);
> goto out;
> }
>
> if (asidentifiers->asnum == NULL) {
> warnx("%s: RFC 6487 section 4.8.11: autonomousSysNum: "
> - "no AS number resource set", p->fn);
> + "no AS number resource set", fn);
> goto out;
> }
>
> @@ -200,26 +186,25 @@ sbgp_assysnum(struct parse *p, X509_EXTE
> break;
> default:
> warnx("%s: RFC 3779 section 3.2.3.2: ASIdentifierChoice: "
> - "unknown type %d", p->fn, asidentifiers->asnum->type);
> + "unknown type %d", fn, asidentifiers->asnum->type);
> goto out;
> }
>
> if (asz == 0) {
> - warnx("%s: RFC 6487 section 4.8.11: empty asIdsOrRanges",
> - p->fn);
> + warnx("%s: RFC 6487 section 4.8.11: empty asIdsOrRanges", fn);
> goto out;
> }
> if (asz >= MAX_AS_SIZE) {
> warnx("%s: too many AS number entries: limit %d",
> - p->fn, MAX_AS_SIZE);
> + fn, MAX_AS_SIZE);
> goto out;
> }
> - p->res->as = calloc(asz, sizeof(struct cert_as));
> - if (p->res->as == NULL)
> + as = calloc(asz, sizeof(struct cert_as));
> + if (as == NULL)
> err(1, NULL);
>
> if (aors == NULL) {
> - if (!sbgp_as_inherit(p->fn, p->res->as, >res->asz))
> + if (!sbgp_as_inherit(fn, as, _asz))
> goto out;
> }
>
> @@ -229,22 +214,58 @@ sbgp_assysnum(struct parse *p, X509_EXTE
> aor = sk_ASIdOrRange_value(aors, i);
> switch (aor->type) {
> case ASIdOrRange_id:
> - if (!sbgp_as_id(p->fn, p->res->as, >res->asz,
> -
Re: rpki-client: refactor sbgp_assysnum() and sbgp_ipaddrblk()
On Mon, Sep 25, 2023 at 02:47:37PM +0200, Claudio Jeker wrote:
> On Sat, Sep 23, 2023 at 01:23:34PM +0200, Theo Buehler wrote:
> > This is a second chunk split out of the diff mentioned in my previous
> > mail. It factors the parsing of ASIdentifiers and IPAddrBlocks out of
> > sbgp_assysnum() and sbgp_ipaddrblk() and makes the latter only extract
> > the info from the X509_EXTENSION. This should not change anything, but
> > the logic is a bit tricky.
> >
> > We could initialize *as and *asz, as well as *ips and *ipsz to NULL/0,
> > at the top of the two new sbgp_parse_*.
>
> It looks inded like nthing is changed. The thing I dislike a bit is how
> **as and *asz are updated inside the sbgp_parse_* functions. There is
> return 0 before and after the calloc / recallocarray calls and so it
> depends a lot on the caller to be careful here. The code right now is ok.
Thanks for that clue. I didn't particularly like my diff either. The
below is better, has less churn and should be easier to review. This way
the caller doesn't have to be careful.
I left the currently existing variables asz and ipsz untouched since it
becomes too confusing. I want to rename asz -> sz and new_asz -> asz in
a follow-up, similarly for ipsz.
Index: cert.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
retrieving revision 1.115
diff -u -p -r1.115 cert.c
--- cert.c 12 Sep 2023 09:33:30 - 1.115
+++ cert.c 25 Sep 2023 14:29:56 -
@@ -153,40 +153,26 @@ sbgp_as_inherit(const char *fn, struct c
return append_as(fn, ases, asz, );
}
-/*
- * Parse RFC 6487 4.8.11 X509v3 extension, with syntax documented in RFC
- * 3779 starting in section 3.2.
- * Returns zero on failure, non-zero on success.
- */
-static int
-sbgp_assysnum(struct parse *p, X509_EXTENSION *ext)
+int
+sbgp_parse_assysnum(const char *fn, const ASIdentifiers *asidentifiers,
+struct cert_as **out_as, size_t *out_asz)
{
- ASIdentifiers *asidentifiers = NULL;
const ASIdOrRanges *aors = NULL;
- size_t asz;
- int i, rc = 0;
+ struct cert_as *as = NULL;
+ size_t asz, new_asz = 0;
+ int i;
- if (!X509_EXTENSION_get_critical(ext)) {
- warnx("%s: RFC 6487 section 4.8.11: autonomousSysNum: "
- "extension not critical", p->fn);
- goto out;
- }
-
- if ((asidentifiers = X509V3_EXT_d2i(ext)) == NULL) {
- warnx("%s: RFC 6487 section 4.8.11: autonomousSysNum: "
- "failed extension parse", p->fn);
- goto out;
- }
+ assert(*out_as == NULL && *out_asz == 0);
if (asidentifiers->rdi != NULL) {
warnx("%s: RFC 6487 section 4.8.11: autonomousSysNum: "
- "should not have RDI values", p->fn);
+ "should not have RDI values", fn);
goto out;
}
if (asidentifiers->asnum == NULL) {
warnx("%s: RFC 6487 section 4.8.11: autonomousSysNum: "
- "no AS number resource set", p->fn);
+ "no AS number resource set", fn);
goto out;
}
@@ -200,26 +186,25 @@ sbgp_assysnum(struct parse *p, X509_EXTE
break;
default:
warnx("%s: RFC 3779 section 3.2.3.2: ASIdentifierChoice: "
- "unknown type %d", p->fn, asidentifiers->asnum->type);
+ "unknown type %d", fn, asidentifiers->asnum->type);
goto out;
}
if (asz == 0) {
- warnx("%s: RFC 6487 section 4.8.11: empty asIdsOrRanges",
- p->fn);
+ warnx("%s: RFC 6487 section 4.8.11: empty asIdsOrRanges", fn);
goto out;
}
if (asz >= MAX_AS_SIZE) {
warnx("%s: too many AS number entries: limit %d",
- p->fn, MAX_AS_SIZE);
+ fn, MAX_AS_SIZE);
goto out;
}
- p->res->as = calloc(asz, sizeof(struct cert_as));
- if (p->res->as == NULL)
+ as = calloc(asz, sizeof(struct cert_as));
+ if (as == NULL)
err(1, NULL);
if (aors == NULL) {
- if (!sbgp_as_inherit(p->fn, p->res->as, >res->asz))
+ if (!sbgp_as_inherit(fn, as, _asz))
goto out;
}
@@ -229,22 +214,58 @@ sbgp_assysnum(struct parse *p, X509_EXTE
aor = sk_ASIdOrRange_value(aors, i);
switch (aor->type) {
case ASIdOrRange_id:
- if (!sbgp_as_id(p->fn, p->res->as, >res->asz,
- aor->u.id))
+ if (!sbgp_as_id(fn, as, _asz, aor->u.id))
goto out;
break;
case
Re: Please test; midi(4): make midi{read,write}_filtops mp safe
On Mon, Sep 25, 2023 at 05:39:34AM +, Visa Hankala wrote:
> On Sun, Sep 24, 2023 at 11:03:54PM +0300, Vitaliy Makkoveev wrote:
> > Please test this diff, I have no midi(4) devices.
> >
> > midi(4) already uses `audio_lock' mutex(9) for filterops, but they are
> > still kernel locked. Wipe out old selwakeup API and make them MP safe.
> > knote_locked(9) will not grab kernel lock, so call it directly from
> > interrupt handlers instead of scheduling software interrupts.
>
> https://marc.info/?l=openbsd-tech=167604232828221 has minor takeaways
> if you pay attention.
>
The only significant difference is mididetach() where you did not replaced
selwakeup() with knote(). Did I missed something?
@@ -577,30 +562,20 @@ mididetach(struct device *self, int flag
* in read/write/ioctl, which return EIO.
*/
if (sc->flags) {
- KERNEL_ASSERT_LOCKED();
- if (sc->flags & FREAD) {
+ if (sc->flags & FREAD)
wakeup(>inbuf.blocking);
- mtx_enter(_lock);
- selwakeup(>inbuf.sel);
- mtx_leave(_lock);
- }
> > Index: sys/dev/midi.c
> > ===
> > RCS file: /cvs/src/sys/dev/midi.c,v
> > retrieving revision 1.55
> > diff -u -p -r1.55 midi.c
> > --- sys/dev/midi.c 2 Jul 2022 08:50:41 - 1.55
> > +++ sys/dev/midi.c 24 Sep 2023 19:57:56 -
> > @@ -31,7 +31,6 @@
> > #include
> > #include
> >
> > -#define IPL_SOFTMIDI IPL_SOFTNET
> > #define DEVNAME(sc)((sc)->dev.dv_xname)
> >
> > intmidiopen(dev_t, int, int, struct proc *);
> > @@ -65,41 +64,38 @@ struct cfdriver midi_cd = {
> >
> > void filt_midiwdetach(struct knote *);
> > int filt_midiwrite(struct knote *, long);
> > +int filt_midimodify(struct kevent *, struct knote *);
> > +int filt_midiprocess(struct knote *, struct kevent *);
> >
> > const struct filterops midiwrite_filtops = {
> > - .f_flags= FILTEROP_ISFD,
> > + .f_flags= FILTEROP_ISFD | FILTEROP_MPSAFE,
> > .f_attach = NULL,
> > .f_detach = filt_midiwdetach,
> > .f_event= filt_midiwrite,
> > + .f_modify = filt_midimodify,
> > + .f_process = filt_midiprocess,
> > };
> >
> > void filt_midirdetach(struct knote *);
> > int filt_midiread(struct knote *, long);
> >
> > const struct filterops midiread_filtops = {
> > - .f_flags= FILTEROP_ISFD,
> > + .f_flags= FILTEROP_ISFD | FILTEROP_MPSAFE,
> > .f_attach = NULL,
> > .f_detach = filt_midirdetach,
> > .f_event= filt_midiread,
> > + .f_modify = filt_midimodify,
> > + .f_process = filt_midiprocess,
> > };
> >
> > void
> > -midi_buf_wakeup(void *addr)
> > +midi_buf_wakeup(struct midi_buffer *buf)
> > {
> > - struct midi_buffer *buf = addr;
> > -
> > if (buf->blocking) {
> > wakeup(>blocking);
> > buf->blocking = 0;
> > }
> > - /*
> > -* As long as selwakeup() grabs the KERNEL_LOCK() make sure it is
> > -* already held here to avoid lock ordering problems with `audio_lock'
> > -*/
> > - KERNEL_ASSERT_LOCKED();
> > - mtx_enter(_lock);
> > - selwakeup(>sel);
> > - mtx_leave(_lock);
> > + knote_locked(>klist, 0);
> > }
> >
> > void
> > @@ -117,13 +113,7 @@ midi_iintr(void *addr, int data)
> >
> > MIDIBUF_WRITE(mb, data);
> >
> > - /*
> > -* As long as selwakeup() needs to be protected by the
> > -* KERNEL_LOCK() we have to delay the wakeup to another
> > -* context to keep the interrupt context KERNEL_LOCK()
> > -* free.
> > -*/
> > - softintr_schedule(sc->inbuf.softintr);
> > + midi_buf_wakeup(mb);
> > }
> >
> > int
> > @@ -226,14 +216,7 @@ void
> > midi_out_stop(struct midi_softc *sc)
> > {
> > sc->isbusy = 0;
> > -
> > - /*
> > -* As long as selwakeup() needs to be protected by the
> > -* KERNEL_LOCK() we have to delay the wakeup to another
> > -* context to keep the interrupt context KERNEL_LOCK()
> > -* free.
> > -*/
> > - softintr_schedule(sc->outbuf.softintr);
> > + midi_buf_wakeup(>outbuf);
> > }
> >
> > void
> > @@ -342,11 +325,11 @@ midikqfilter(dev_t dev, struct knote *kn
> > error = 0;
> > switch (kn->kn_filter) {
> > case EVFILT_READ:
> > - klist = >inbuf.sel.si_note;
> > + klist = >inbuf.klist;
> > kn->kn_fop = _filtops;
> > break;
> > case EVFILT_WRITE:
> > - klist = >outbuf.sel.si_note;
> > + klist = >outbuf.klist;
> > kn->kn_fop = _filtops;
> > break;
> > default:
> > @@ -355,9 +338,7 @@ midikqfilter(dev_t dev, struct knote *kn
> > }
> > kn->kn_hook = (void *)sc;
> >
> > - mtx_enter(_lock);
> > - klist_insert_locked(klist, kn);
> > - mtx_leave(_lock);
> > +
Re: [patch] [arm64] cpu.c patch based on amd64 idea, provides more debug for multicore kernel
On Tue, Jul 25, 2023 at 01:30:43PM +0300, Slava Voronzoff wrote: > Hi, pinging and refreshing this patch > > What it does: > allow arm64 cpus to break from the loop of waiting to start core and > drop to DDB or OS. > > Patch based on same concept in amd64 cpu.c > > Any suggestions? Good to go? So instead of waiting possibly forever for secondary CPUs to come up, you can continue debug the system and/or continue boot with less CPUs. Apart from the trailing empty line you introduce, the approach does match amd64 (down to the for loop lacking a space after semicolon). That allows making progress on these machines and I don't see a downside, so OK kn modulo the empty line. Any input from our arm64 hackers? Same diff ten days ago: https://marc.info/?l=openbsd-bugs=169465443200821=2
Re: Send international text with mail(1) - proposal and patches
This new version, when it detects invalid utf-8 in the body saves a
dead.letter, prints the following message and exits.
$ mail -s hello user < invalid_utf8.txt
Invalid or incomplete multibyte or wide character
. . . message not sent.
Index: send.c
===
RCS file: /cvs/src/usr.bin/mail/send.c,v
retrieving revision 1.26
diff -u -p -r1.26 send.c
--- send.c 8 Mar 2023 04:43:11 - 1.26
+++ send.c 25 Sep 2023 13:07:17 -
@@ -32,6 +32,11 @@
#include "rcv.h"
#include "extern.h"
+#include "locale.h"
+
+/* To check charset of the message and add the appropiate MIME headers */
+static char nutf8;
+static int not_utf8(FILE *s, int len);
static volatile sig_atomic_t sendsignal; /* Interrupted by a signal? */
@@ -341,6 +346,17 @@ mail1(struct header *hp, int printheader
else
puts("Null message body; hope that's ok");
}
+
+ /* Check non valid UTF-8 characters in the message */
+ nutf8 = not_utf8(mtf, fsize(mtf));
+ rewind(mtf);
+ if (nutf8 > 1) {
+ savedeadletter(mtf);
+ puts("Invalid or incomplete multibyte or wide character");
+ fputs(". . . message not sent.\n", stderr);
+ exit(1);
+ }
+
/*
* Now, take the user names from the combined
* to and cc lists and do all the alias
@@ -520,15 +536,30 @@ puthead(struct header *hp, FILE *fo, int
gotcha = 0;
from = hp->h_from ? hp->h_from : value("from");
if (from != NULL)
- fprintf(fo, "From: %s\n", from), gotcha++;
+ fprintf(fo, "From: %s\n", from),
+ gotcha++;
if (hp->h_to != NULL && w & GTO)
- fmt("To:", hp->h_to, fo, w), gotcha++;
+ fmt("To:", hp->h_to, fo, w),
+ gotcha++;
if (hp->h_subject != NULL && w & GSUBJECT)
- fprintf(fo, "Subject: %s\n", hp->h_subject), gotcha++;
+ fprintf(fo, "Subject: %s\n", hp->h_subject),
+ gotcha++;
+ if (nutf8 == 0)
+ fprintf(fo, "MIME-Version: 1.0\n"
+ "Content-Type: text/plain; charset=us-ascii\n"
+ "Content-Transfer-Encoding: 7bit\n"),
+ gotcha++;
+ else if (nutf8 == 1)
+ fprintf(fo, "MIME-Version: 1.0\n"
+ "Content-Type: text/plain; charset=utf-8\n"
+ "Content-Transfer-Encoding: 8bit\n"),
+ gotcha++;
if (hp->h_cc != NULL && w & GCC)
- fmt("Cc:", hp->h_cc, fo, w), gotcha++;
+ fmt("Cc:", hp->h_cc, fo, w),
+ gotcha++;
if (hp->h_bcc != NULL && w & GBCC)
- fmt("Bcc:", hp->h_bcc, fo, w), gotcha++;
+ fmt("Bcc:", hp->h_bcc, fo, w),
+ gotcha++;
if (gotcha && w & GNL)
(void)putc('\n', fo);
return(0);
@@ -609,4 +640,44 @@ sendint(int s)
{
sendsignal = s;
+}
+
+/* Search non valid UTF-8 characters in the message */
+static int
+not_utf8(FILE *message, int len)
+{
+ int c, count, n, ulen;
+ size_t i, resize;
+ size_t jump = 100;
+ unsigned char *s = NULL;
+
+ setlocale(LC_CTYPE, "en_US.UTF-8");
+
+ if (s == NULL && (s = malloc(jump)) == NULL)
+ err(1, NULL);
+
+ i = count = 0;
+ while ((c = getc(message)) != EOF) {
+ if (s == NULL || count == jump) {
+ if ((s = realloc(s, i + jump + 1)) == NULL)
+ err(1, NULL);
+ count = 0;
+ }
+ s[i++] = c;
+ count++;
+ }
+
+ s[i] = '\0';
+
+ ulen = mbstowcs(NULL, s, 0);
+
+ if (ulen == len)
+ n = 0;
+ else if (ulen < 0)
+ n = 2;
+ else if (ulen < len)
+ n = 1;
+
+ free(s);
+ return n;
}
--
Walter
Re: prevent re-upgrade in powerpc64 boot loader
On Mon, Sep 25, 2023 at 05:35:40AM +, Visa Hankala wrote:
> On Sat, Sep 23, 2023 at 02:26:18PM +, Klemens Nanni wrote:
> > On Sat, Sep 23, 2023 at 01:11:32PM +0200, Mark Kettenis wrote:
> > > > Date: Thu, 21 Sep 2023 22:30:01 +
> > > > From: Klemens Nanni
> > > >
> > > > In comparison to MI boot which only cares about /bsd.upgrade's x bit,
> > > > powerpc64 rdboot just wants a regular file.
> > > >
> > > > Require and strip u+x before execution to prevent sysupgrade(8) loop.
> > > > I'm new to powerpc64 and can't think of a reason to be different.
> > > >
> > > > Feedback? Objection? OK?
> > >
> > > So there is a problem with this approach. Calling chmod() will mean
> > > the bootloader will change the filesystem. What happens if you're
> > > booting with a root filesystem that was not cleanly unmounted?
> > > Modifying a forcibly mounted filesystem may not be without risk.
> >
> > Isn't that already the case with chmo() /etc/random.seed?
> >
> > Can you explain how that is not a problem in other non-kernel boot loaders
> > using libsa's ufs2_open() instead of mount(2)?
>
> chmod() through libsa's filesystem code modifies only the inode.
> Doing a mount-chmod-unmount cycle using the kernel triggers multiple
> writes to the filesystem.
>
> In the past, I have pondered if octeon (and powerpc64) bootcode should
> use libsa instead of replicating the MI boot code. I did not use libsa
> initially because the libsa and libc APIs differ, and I did not want
> to use custom system call stubs. The original octboot/kexec interface
> was not suitable for libsa use, either.
Are there more reasons or benefits to this approach than a) less intrusive
chmod() and b) more shared, overall less code?
I obviously don't see the full picture (yet).
> However, the libsa and libc worlds can be reconciled with a trick.
> The libsa code is first compiled in standalone mode into a relinkable
> object ("saboot"). This object is then tweaked to avoid name conflicts
> with libc. Finally, the object is linked with glue code and libc into
> a proper rdboot executable that the kernel can run.
>
> Some have seen this before.
>
>
> diff --git a/sys/arch/octeon/stand/Makefile b/sys/arch/octeon/stand/Makefile
> index 498bd2809c6..4a86a8bef6b 100644
> --- a/sys/arch/octeon/stand/Makefile
> +++ b/sys/arch/octeon/stand/Makefile
> @@ -1,5 +1,5 @@
> #$OpenBSD: Makefile,v 1.4 2019/08/04 08:53:14 visa Exp $
>
> -SUBDIR+= rdboot boot
> +SUBDIR+= saboot rdboot boot
>
> .include
> diff --git a/sys/arch/octeon/stand/Makefile.inc
> b/sys/arch/octeon/stand/Makefile.inc
> deleted file mode 100644
> index 18d8470f888..000
> --- a/sys/arch/octeon/stand/Makefile.inc
> +++ /dev/null
> @@ -1,45 +0,0 @@
> -#$OpenBSD: Makefile.inc,v 1.1 2013/06/05 01:02:29 jasper Exp $
> -
> -BINDIR= /usr/mdec
> -
> -STANDALONE?= -D_STANDALONE
> -
> -.if ${MACHINE} == "octeon"
> -CPPFLAGS+= ${STANDALONE}
> -CPPFLAGS+= -I.
> -CFLAGS+= -fno-stack-protector -Wall
> -CFLAGS+= -fno-builtin-vprintf -fno-builtin-printf -fno-builtin-putchar
> -# Silence warnings
> -CFLAGS+= -fno-builtin-snprintf
> -CFLAGS+= -fno-builtin-memcpy
> -CFLAGS+= -fno-builtin-memcmp
> -CFLAGS+= -fno-builtin-memset
> -CFLAGS+= -fno-builtin-strncpy
> -CFLAGS+= -fno-builtin-strncmp
> -CFLAGS+= -fno-builtin-exit
> -SAABI= -mips3 -mno-abicalls -G 0 -fno-pic -fno-common
> -AS?= as
> -LD?= ld
> -.endif
> -
> -### Figure out what to use for libsa
> -LIBSADIR?= ${.CURDIR}/../libsa
> -
> -.if exists(${LIBSADIR}/${__objdir})
> -LIBSAOBJDIR=${LIBSADIR}/${__objdir}
> -.else
> -LIBSAOBJDIR=${LIBSADIR}
> -.endif
> -
> -LIBSA= ${LIBSAOBJDIR}/libsa.a
> -
> -### Figure out what to use for libz
> -LIBZDIR?=${.CURDIR}/../libz
> -
> -.if exists(${LIBZDIR}/${__objdir})
> -LIBZOBJDIR= ${LIBZDIR}/${__objdir}
> -.else
> -LIBZOBJDIR= ${LIBZDIR}
> -.endif
> -
> -LIBZ= ${LIBZOBJDIR}/libz.a
> diff --git a/sys/arch/octeon/stand/libsa/Makefile
> b/sys/arch/octeon/stand/libsa/Makefile
> deleted file mode 100644
> index 3b2961f25c0..000
> --- a/sys/arch/octeon/stand/libsa/Makefile
> +++ /dev/null
> @@ -1,51 +0,0 @@
> -#$OpenBSD: Makefile,v 1.8 2019/10/29 02:55:52 deraadt Exp $
> -
> -.include "${.CURDIR}/../Makefile.inc"
> -
> -LIB= sa
> -
> -.PATH: ${.CURDIR}/../../../../lib/libsa
> -
> -CLEANFILES += machine mips64
> -
> -CFLAGS+= ${CEXTRAFLAGS} ${SAABI} -nostdinc -mno-abicalls -D_NO_ABICALLS \
> - -fno-pie \
> - -I${.CURDIR} -I${.CURDIR}/../include -I${.CURDIR}/../.. \
> - -I${.CURDIR}/../../.. -I${.CURDIR}/../../../.. \
> - -I${.CURDIR}/../../../../lib/libsa \
> - -I${.OBJDIR}
> -
> -# stand routines
> -SRCS=alloc.c cons.c ctime.c exit.c getchar.c getfile.c getln.c
> globals.c \
> - memcmp.c memcpy.c memmove.c memset.c printf.c
Re: rpki-client: refactor sbgp_assysnum() and sbgp_ipaddrblk()
On Sat, Sep 23, 2023 at 01:23:34PM +0200, Theo Buehler wrote:
> This is a second chunk split out of the diff mentioned in my previous
> mail. It factors the parsing of ASIdentifiers and IPAddrBlocks out of
> sbgp_assysnum() and sbgp_ipaddrblk() and makes the latter only extract
> the info from the X509_EXTENSION. This should not change anything, but
> the logic is a bit tricky.
>
> We could initialize *as and *asz, as well as *ips and *ipsz to NULL/0,
> at the top of the two new sbgp_parse_*.
It looks inded like nthing is changed. The thing I dislike a bit is how
**as and *asz are updated inside the sbgp_parse_* functions. There is
return 0 before and after the calloc / recallocarray calls and so it
depends a lot on the caller to be careful here. The code right now is ok.
One minor nit though:
> Index: cert.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
> retrieving revision 1.115
> diff -u -p -r1.115 cert.c
> --- cert.c12 Sep 2023 09:33:30 - 1.115
> +++ cert.c23 Sep 2023 11:03:48 -
> +/*
> + * Parse RFC 6487 4.8.11 X509v3 extension, with syntax documented in RFC
> + * 3779 starting in section 3.2.
> + * Returns zero on failure, non-zero on success.
> + */
> +static int
> +sbgp_assysnum(struct parse *p, X509_EXTENSION *ext)
> +{
> + ASIdentifiers *asidentifiers = NULL;
> + int rc = 0;
> +
> + if (!X509_EXTENSION_get_critical(ext)) {
> + warnx("%s: RFC 6487 section 4.8.11: autonomousSysNum: "
> + "extension not critical", p->fn);
> + goto out;
> + }
> +
> + if ((asidentifiers = X509V3_EXT_d2i(ext)) == NULL) {
> + warnx("%s: RFC 6487 section 4.8.11: autonomousSysNum: "
> + "failed extension parse", p->fn);
> + goto out;
> + }
> +
> + if (!sbgp_parse_assysnum(p->fn, asidentifiers, >res->as,
> >res->asz))
This line is over 80 chars.
Apart from that OK.
--
:wq Claudio
Re: rpki-client: pass talid to callers of cert_parse_ee_cert()
On Sat, Sep 23, 2023 at 12:59:48PM +0200, Theo Buehler wrote:
> This is a boring mechanical diff that splits some of the noise out of a
> larger diff that Job will send out and explain in detail soon. In itself
> it changes nothing. For a given product we will need to know the
> originating TA for additional checks in cert_parse_ee_cert().
>
> The callers of cert_parse_ee_cert() are *_parse(), except cert_parse()
> and crl_parse(), which are special anyway.
>
> In !filemode the talid is known to the caller of proc_parser_* (since
> struct entp contains it). proc_parser_* later recovers this info from
> struct auth returned by valid_ski_aki() but that's only possible after
> *_parse() was called.
>
> So pass the full struct entp * to proc_parser_*() instead of only the
> entp->mftaki and then pass entp->talid and entp->mftaki where needed.
>
> In filemode the talid is unknown at the point when *_parse() is called,
> so set it to -1 to indicate that.
>
> There are various other ways of achieving what Job's plan needs. For
> example, we could replace X509 ** with struct cert_ip ** in *_parse and
> do the check in proc_parser_* instead of cert_parse_ee_cert(). The
> resulting complexity is about the same and unless there are strong
> concerns or objections I'd like to do it the way below.
>
> Regress needs a trivial adjustment that I will commit at the same time.
I see nothing that speaks against this. OK claudio@
> Index: aspa.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/aspa.c,v
> retrieving revision 1.22
> diff -u -p -r1.22 aspa.c
> --- aspa.c10 Jul 2023 12:02:37 - 1.22
> +++ aspa.c23 Sep 2023 09:59:32 -
> @@ -159,7 +159,8 @@ aspa_parse_econtent(const unsigned char
> * Returns the payload or NULL if the file was malformed.
> */
> struct aspa *
> -aspa_parse(X509 **x509, const char *fn, const unsigned char *der, size_t len)
> +aspa_parse(X509 **x509, const char *fn, int talid, const unsigned char *der,
> +size_t len)
> {
> struct parse p;
> size_t cmsz;
> Index: extern.h
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v
> retrieving revision 1.189
> diff -u -p -r1.189 extern.h
> --- extern.h 12 Sep 2023 09:33:30 - 1.189
> +++ extern.h 23 Sep 2023 09:59:32 -
> @@ -624,33 +624,33 @@ void cert_insert_brks(struct brk_tree
> enum rtypertype_from_file_extension(const char *);
> void mft_buffer(struct ibuf *, const struct mft *);
> void mft_free(struct mft *);
> -struct mft *mft_parse(X509 **, const char *, const unsigned char *,
> +struct mft *mft_parse(X509 **, const char *, int, const unsigned char *,
> size_t);
> struct mft *mft_read(struct ibuf *);
> int mft_compare(const struct mft *, const struct mft *);
>
> void roa_buffer(struct ibuf *, const struct roa *);
> void roa_free(struct roa *);
> -struct roa *roa_parse(X509 **, const char *, const unsigned char *,
> +struct roa *roa_parse(X509 **, const char *, int, const unsigned char *,
> size_t);
> struct roa *roa_read(struct ibuf *);
> void roa_insert_vrps(struct vrp_tree *, struct roa *,
> struct repo *);
>
> void gbr_free(struct gbr *);
> -struct gbr *gbr_parse(X509 **, const char *, const unsigned char *,
> +struct gbr *gbr_parse(X509 **, const char *, int, const unsigned char *,
> size_t);
>
> void geofeed_free(struct geofeed *);
> -struct geofeed *geofeed_parse(X509 **, const char *, char *, size_t);
> +struct geofeed *geofeed_parse(X509 **, const char *, int, char *,
> size_t);
>
> void rsc_free(struct rsc *);
> -struct rsc *rsc_parse(X509 **, const char *, const unsigned char *,
> +struct rsc *rsc_parse(X509 **, const char *, int, const unsigned char *,
> size_t);
>
> void takey_free(struct takey *);
> void tak_free(struct tak *);
> -struct tak *tak_parse(X509 **, const char *, const unsigned char *,
> +struct tak *tak_parse(X509 **, const char *, int, const unsigned char *,
> size_t);
> struct tak *tak_read(struct ibuf *);
>
> @@ -658,7 +658,7 @@ void aspa_buffer(struct ibuf *, const
> void aspa_free(struct aspa *);
> void aspa_insert_vaps(struct vap_tree *, struct aspa *,
> struct repo *);
> -struct aspa *aspa_parse(X509 **, const char *, const unsigned char *,
> +struct aspa *aspa_parse(X509 **, const char *, int, const unsigned char *,
> size_t);
> struct aspa *aspa_read(struct ibuf *);
>
> Index: filemode.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/filemode.c,v
> retrieving revision 1.34
> diff -u
