Re: patch: make ld.so aware of pthread_key_create destructor - Re: multimedia/mpv debug vo=gpu crash on exit
On Thu, May 06, 2021 at 06:23:08PM -0700, Anindya Mukherjee wrote: > On Thu, May 06, 2021 at 08:00:56AM -0600, Todd C. Miller wrote: > > On Thu, 06 May 2021 09:32:28 +0200, Sebastien Marie wrote: > > > > > We already take care of such situation with __cxa_thread_atexit_impl > > > (in libc/stdlib/thread_atexit.c), by keeping an additionnal reference > > > on object loaded (it makes ld.so aware that it is still used and so > > > dlclose() doesn't unload it). > > > > > > I used the same idiom for pthread_key_create() and used dlctl(3) in > > > the same way with the destructor address. > > > > This will set STAT_NODELETE so the DSO will never really get unloaded. > > That's not a problem for atexit() since the process is headed for > > the exit. > > > > I'm less sure about using it here since we don't have a way to > > unreference the DSO upon pthread_key_delete(). > > > > - todd > > I did a quick investigation on my Linux machine and there mpv seems to > be using libEGL_mesa.so instead of iris_dri.so. In this case I am not > seeing a call to pthread_key_create at the start of video playback > (there are some other places where pthread_key_create is called from but > they don't cause a problem). So, not sure what happens in Linux when > iris_dri.so is used. libEGL_mesa.so seems to be used when mesa is built with 'with_glvnd' option. glvnd is "vendor-neutral libGL" : https://gitlab.freedesktop.org/glvnd/libglvnd > However, the Linux implementation of > pthread_key_create seems to also not increment the refcount when the > destructor is set so I don't yet see how it's solved there, assuming > iris_dri.so behaves identically. glibc seems to have the same problem with pthread_key_create(): https://sourceware.org/bugzilla/show_bug.cgi?id=21032 and the bugreport reference a simple poc at https://github.com/Aaron1011/pthread_dlopen -- Sebastien Marie
Re: patch: make ld.so aware of pthread_key_create destructor - Re: multimedia/mpv debug vo=gpu crash on exit
On Fri, May 07, 2021 at 04:41:50PM -0700, Anindya Mukherjee wrote: > On Thu, May 06, 2021 at 08:00:56AM -0600, Todd C. Miller wrote: > > On Thu, 06 May 2021 09:32:28 +0200, Sebastien Marie wrote: > > > > > We already take care of such situation with __cxa_thread_atexit_impl > > > (in libc/stdlib/thread_atexit.c), by keeping an additionnal reference > > > on object loaded (it makes ld.so aware that it is still used and so > > > dlclose() doesn't unload it). > > > > > > I used the same idiom for pthread_key_create() and used dlctl(3) in > > > the same way with the destructor address. > > > > This will set STAT_NODELETE so the DSO will never really get unloaded. > > That's not a problem for atexit() since the process is headed for > > the exit. > > > > I'm less sure about using it here since we don't have a way to > > unreference the DSO upon pthread_key_delete(). > > For sure I don't fully appreciate the complexities involved here, but is > it possible to store the shared object handle along with the destructor > when the reference count is incremented in the above patch? Then we > could use that to decrement the reference. > It needs help from ld.so to decrement the reference and unload the DSO if the refcount is 0. It is something doable (by extending dlctl(3) command set), but it could be tricky to implement properly. I would be help full to understand why EGL is using some code path on Linux and another on OpenBSD: it would permit to use a more common/tested code path and avoid the issue at first. Thanks. -- Sebastien Marie
Re: patch: make ld.so aware of pthread_key_create destructor - Re: multimedia/mpv debug vo=gpu crash on exit
On Thu, May 06, 2021 at 08:00:56AM -0600, Todd C. Miller wrote: > On Thu, 06 May 2021 09:32:28 +0200, Sebastien Marie wrote: > > > We already take care of such situation with __cxa_thread_atexit_impl > > (in libc/stdlib/thread_atexit.c), by keeping an additionnal reference > > on object loaded (it makes ld.so aware that it is still used and so > > dlclose() doesn't unload it). > > > > I used the same idiom for pthread_key_create() and used dlctl(3) in > > the same way with the destructor address. > > This will set STAT_NODELETE so the DSO will never really get unloaded. > That's not a problem for atexit() since the process is headed for > the exit. > > I'm less sure about using it here since we don't have a way to > unreference the DSO upon pthread_key_delete(). For sure I don't fully appreciate the complexities involved here, but is it possible to store the shared object handle along with the destructor when the reference count is incremented in the above patch? Then we could use that to decrement the reference. > > - todd Thanks for looking into this. Regards, Anindya
Re: more rsync cleanup
Claudio Jeker(cje...@diehard.n-r-g.com) on 2021.05.06 17:59:32 +0200:
> As noticed by benno@ the blk.blks buffer is leaked in some cases.
> Fix those and cleanup up the pre_* functions a bit more.
> I increased the diff context a bit to make the diff easier to read.
reads ok
>
> --
> :wq Claudio
>
> Index: uploader.c
> ===
> RCS file: /cvs/src/usr.bin/rsync/uploader.c,v
> retrieving revision 1.25
> diff -u -p -U6 -r1.25 uploader.c
> --- uploader.c6 May 2021 07:35:22 - 1.25
> +++ uploader.c6 May 2021 15:34:36 -
> @@ -191,22 +191,24 @@ pre_link(struct upload *p, struct sess *
>* to overwriting with a symbolic link.
>* If it's a non-directory, we just overwrite it.
>*/
>
> assert(p->rootfd != -1);
> rc = fstatat(p->rootfd, f->path, &st, AT_SYMLINK_NOFOLLOW);
> +
> + if (rc == -1 && errno != ENOENT) {
> + ERR("%s: fstatat", f->path);
> + return -1;
> + }
> if (rc != -1 && !S_ISLNK(st.st_mode)) {
> if (S_ISDIR(st.st_mode) &&
> unlinkat(p->rootfd, f->path, AT_REMOVEDIR) == -1) {
> ERR("%s: unlinkat", f->path);
> return -1;
> }
> rc = -1;
> - } else if (rc == -1 && errno != ENOENT) {
> - ERR("%s: fstatat", f->path);
> - return -1;
> }
>
> /*
>* If the symbolic link already exists, then make sure that it
>* points to the correct place.
>*/
> @@ -294,22 +296,23 @@ pre_dev(struct upload *p, struct sess *s
>* If it replaces a directory, remove the directory first.
>*/
>
> assert(p->rootfd != -1);
> rc = fstatat(p->rootfd, f->path, &st, AT_SYMLINK_NOFOLLOW);
>
> + if (rc == -1 && errno != ENOENT) {
> + ERR("%s: fstatat", f->path);
> + return -1;
> + }
> if (rc != -1 && !(S_ISBLK(st.st_mode) || S_ISCHR(st.st_mode))) {
> if (S_ISDIR(st.st_mode) &&
> unlinkat(p->rootfd, f->path, AT_REMOVEDIR) == -1) {
> ERR("%s: unlinkat", f->path);
> return -1;
> }
> rc = -1;
> - } else if (rc == -1 && errno != ENOENT) {
> - ERR("%s: fstatat", f->path);
> - return -1;
> }
>
> /* Make sure existing device is of the correct type. */
>
> if (rc != -1) {
> if ((f->st.mode & (S_IFCHR|S_IFBLK)) !=
> @@ -382,22 +385,23 @@ pre_fifo(struct upload *p, struct sess *
>* mark it from replacement.
>*/
>
> assert(p->rootfd != -1);
> rc = fstatat(p->rootfd, f->path, &st, AT_SYMLINK_NOFOLLOW);
>
> + if (rc == -1 && errno != ENOENT) {
> + ERR("%s: fstatat", f->path);
> + return -1;
> + }
> if (rc != -1 && !S_ISFIFO(st.st_mode)) {
> if (S_ISDIR(st.st_mode) &&
> unlinkat(p->rootfd, f->path, AT_REMOVEDIR) == -1) {
> ERR("%s: unlinkat", f->path);
> return -1;
> }
> rc = -1;
> - } else if (rc == -1 && errno != ENOENT) {
> - ERR("%s: fstatat", f->path);
> - return -1;
> }
>
> if (rc == -1) {
> newfifo = 1;
> if (mktemplate(&temp, f->path, sess->opts->recursive) == -1) {
> ERRX1("mktemplate");
> @@ -458,22 +462,23 @@ pre_sock(struct upload *p, struct sess *
>* mark it from replacement.
>*/
>
> assert(p->rootfd != -1);
> rc = fstatat(p->rootfd, f->path, &st, AT_SYMLINK_NOFOLLOW);
>
> + if (rc == -1 && errno != ENOENT) {
> + ERR("%s: fstatat", f->path);
> + return -1;
> + }
> if (rc != -1 && !S_ISSOCK(st.st_mode)) {
> if (S_ISDIR(st.st_mode) &&
> unlinkat(p->rootfd, f->path, AT_REMOVEDIR) == -1) {
> ERR("%s: unlinkat", f->path);
> return -1;
> }
> rc = -1;
> - } else if (rc == -1 && errno != ENOENT) {
> - ERR("%s: fstatat", f->path);
> - return -1;
> }
>
> if (rc == -1) {
> newsock = 1;
> if (mktemplate(&temp, f->path, sess->opts->recursive) == -1) {
> ERRX1("mktemplate");
> @@ -530,13 +535,14 @@ pre_dir(const struct upload *p, struct s
> assert(p->rootfd != -1);
> rc = fstatat(p->rootfd, f->path, &st, AT_SYMLINK_NOFOLLOW);
>
> if (rc == -1 && errno != ENOENT) {
> ERR("%s: fstatat", f->path);
> return -1;
> - } else if (rc != -1 && !S_ISDIR(st.st_mode)) {
> + }
> + if (rc != -1 && !S_ISDIR(st.st_mode)) {
> ERRX("%s: not a directory", f->path);
> return -1;
> } else if (rc != -1
Re: softraid(4) crypto/raid1c refactoring
The few test reports I've received for this are looking fine.
Would any developer dare to OK this?
On Tue, Apr 27, 2021 at 03:46:56PM +0200, Stefan Sperling wrote:
> Refactor softraid crypto code to allow use of a discipline-specific data
> structure for RAID1C volumes, as requested by jsing@ during review of my
> initial RAID1C patch.
>
> This patch should effectively be a cosmetic change.
> The whole point of this patch is to allow the data structure changes
> made here in softraidvar.h.
>
> It works in my testing but more testing would be very welcome, given
> that this touches the disk I/O path of machines using softraid crypto.
>
> ok?
>
> diff d5cea33885618bf7e096efc36fffbecc9b13ed21
> fcfd3d6487eca3ffe994e6a46e37df66b37e80d1
> blob - d143a2398b5aba3070dc25bafd02e38f8f10a0c1
> blob + 48bd613f374bc6ad085ae5d9e0eeef50a6367941
> --- sys/dev/softraid_crypto.c
> +++ sys/dev/softraid_crypto.c
> @@ -54,30 +54,44 @@
>
> #include
>
> -struct sr_crypto_wu *sr_crypto_prepare(struct sr_workunit *, int);
> -int sr_crypto_create_keys(struct sr_discipline *);
> -int sr_crypto_get_kdf(struct bioc_createraid *,
> - struct sr_discipline *);
> +struct sr_crypto_wu *sr_crypto_prepare(struct sr_workunit *,
> + struct sr_crypto *, int);
> int sr_crypto_decrypt(u_char *, u_char *, u_char *, size_t, int);
> int sr_crypto_encrypt(u_char *, u_char *, u_char *, size_t, int);
> -int sr_crypto_decrypt_key(struct sr_discipline *);
> +int sr_crypto_decrypt_key(struct sr_discipline *,
> + struct sr_crypto *);
> int sr_crypto_change_maskkey(struct sr_discipline *,
> - struct sr_crypto_kdfinfo *, struct sr_crypto_kdfinfo *);
> + struct sr_crypto *, struct sr_crypto_kdfinfo *,
> + struct sr_crypto_kdfinfo *);
> int sr_crypto_create(struct sr_discipline *,
> struct bioc_createraid *, int, int64_t);
> int sr_crypto_meta_create(struct sr_discipline *,
> - struct bioc_createraid *);
> + struct sr_crypto *, struct bioc_createraid *);
> +int sr_crypto_set_key(struct sr_discipline *, struct sr_crypto *,
> + struct bioc_createraid *, int, void *);
> int sr_crypto_assemble(struct sr_discipline *,
> struct bioc_createraid *, int, void *);
> +void sr_crypto_free_sessions(struct sr_discipline *,
> + struct sr_crypto *);
> +int sr_crypto_alloc_resources_internal(struct sr_discipline *,
> + struct sr_crypto *);
> int sr_crypto_alloc_resources(struct sr_discipline *);
> +void sr_crypto_free_resources_internal(struct sr_discipline *,
> + struct sr_crypto *);
> void sr_crypto_free_resources(struct sr_discipline *);
> +int sr_crypto_ioctl_internal(struct sr_discipline *,
> + struct sr_crypto *, struct bioc_discipline *);
> int sr_crypto_ioctl(struct sr_discipline *,
> struct bioc_discipline *);
> +int sr_crypto_meta_opt_handler_internal(struct sr_discipline *,
> + struct sr_crypto *, struct sr_meta_opt_hdr *);
> int sr_crypto_meta_opt_handler(struct sr_discipline *,
> struct sr_meta_opt_hdr *);
> void sr_crypto_write(struct cryptop *);
> int sr_crypto_rw(struct sr_workunit *);
> int sr_crypto_dev_rw(struct sr_workunit *, struct sr_crypto_wu *);
> +void sr_crypto_done_internal(struct sr_workunit *,
> + struct sr_crypto *);
> void sr_crypto_done(struct sr_workunit *);
> void sr_crypto_read(struct cryptop *);
> void sr_crypto_calculate_check_hmac_sha1(u_int8_t *, int,
> @@ -85,7 +99,7 @@ void
> sr_crypto_calculate_check_hmac_sha1(u_int8_t *,
> void sr_crypto_hotplug(struct sr_discipline *, struct disk *, int);
>
> #ifdef SR_DEBUG0
> -void sr_crypto_dumpkeys(struct sr_discipline *);
> +void sr_crypto_dumpkeys(struct sr_crypto *);
> #endif
>
> /* Discipline initialisation. */
> @@ -129,7 +143,7 @@ sr_crypto_create(struct sr_discipline *sd, struct bioc
>
> sd->sd_meta->ssdi.ssd_size = coerced_size;
>
> - rv = sr_crypto_meta_create(sd, bc);
> + rv = sr_crypto_meta_create(sd, &sd->mds.mdd_crypto, bc);
> if (rv)
> return (rv);
>
> @@ -138,7 +152,8 @@ sr_crypto_create(struct sr_discipline *sd, struct bioc
> }
>
> int
> -sr_crypto_meta_create(struct sr_discipline *sd, struct bioc_createraid *bc)
> +sr_crypto_meta_create(struct sr_discipline *sd, struct sr_crypto *mdd_crypto,
> +struct bioc_createraid *bc)
> {
> struct sr_meta_opt_item *omi;
> int rv = EINVAL;
> @@ -158,19 +173,19 @@ sr_crypto_meta_create(struct sr_discipline *sd, struct
> omi->omi_som->som_type =
Re: services(5): add default ftps ports
On Thu, May 06, 2021 at 11:09:03AM -0600, Theo de Raadt wrote: > Jan Klemkow wrote: > > > > > > I'm working on a diff to bring ftps with libtls into our ftpd(8). > > > > > There > > > > > is a "getaddrinfo(NULL, "ftps", &hints, &res0)" call, which uses this > > > > > port. Thus, I made this change. > > > > > > > > Hang on -- does the world want ftps support? > > > > I don't know, what "the world" wants. But, I want ftps. As far as I > > can see, ftps is the only way to bring our ftpd(8) into the 21st > > century. > > I have a really hard time with that. > > The protocol is completely broken, and in a way that adding TLS makes it > even worse. OK. And what should we do with ftpd(8)? I see just three ways: 1. Prepare it for usage in modern internet with crypto support. 2. Just use it for anonymous public file distribution. 3. Remove the daemon. In my opinion the protocol is not that bad and our daemon just need some refactoring and encryption support.
Re: citrus_none: don't allow codepoints > 0x7f
On Fri, 07 May 2021 16:47:19 +0200, Martijn van Duren wrote: > ping Restricting things to 7-bit ASCII in the C/POSIX locale seems like the correct thing to do. OK millert@ - todd
rsync fix file handling in uploader
So yesterday I committed a change to simplify file handling. This removed
the O_NONBLOCK flag from openat() but today I realized that this was a bit
premature. The code at that point does not know if the file is actually a
regular file and so if you put a fifo in place of a regular file it will
hang rsync.
Anyway doing a blind open of any file here is far from ideal. There may be
involuntary side-effects especially on device nodes. So better change the
code to do the fstatat() first and only open the file if it is indeed a
regular file.
While there change log_link to log_symlink which is more precise and also
fix a comment that missed some context.
OK?
--
:wq Claudio
Index: uploader.c
===
RCS file: /cvs/src/usr.bin/rsync/uploader.c,v
retrieving revision 1.25
diff -u -p -r1.25 uploader.c
--- uploader.c 6 May 2021 07:35:22 - 1.25
+++ uploader.c 7 May 2021 15:03:03 -
@@ -79,7 +79,7 @@ log_dir(struct sess *sess, const struct
* operator that we're a link.
*/
static void
-log_link(struct sess *sess, const struct flist *f)
+log_symlink(struct sess *sess, const struct flist *f)
{
if (!sess->opts->server)
@@ -181,7 +181,7 @@ pre_link(struct upload *p, struct sess *
return 0;
}
if (sess->opts->dry_run) {
- log_link(sess, f);
+ log_symlink(sess, f);
return 0;
}
@@ -259,7 +259,7 @@ pre_link(struct upload *p, struct sess *
free(temp);
}
- log_link(sess, f);
+ log_symlink(sess, f);
return 0;
}
@@ -645,6 +645,7 @@ pre_file(const struct upload *p, int *fi
struct sess *sess)
{
const struct flist *f;
+ int rc;
f = &p->fl[p->idx];
assert(S_ISREG(f->st.mode));
@@ -661,51 +662,43 @@ pre_file(const struct upload *p, int *fi
/*
* For non dry-run cases, we'll write the acknowledgement later
* in the rsync_uploader() function.
-* If the call to openat() fails with ENOENT, there's a
-* fast-path between here and the write function.
*/
- *filefd = openat(p->rootfd, f->path,
- O_RDONLY | O_NOFOLLOW, 0);
+ *filefd = -1;
+ rc = fstatat(p->rootfd, f->path, st, AT_SYMLINK_NOFOLLOW);
- if (*filefd == -1 && errno != ENOENT) {
- ERR("%s: openat", f->path);
+ if (rc == -1) {
+ if (errno == ENOENT)
+ return 1;
+
+ ERR("%s: fstatat", f->path);
return -1;
}
- if (*filefd == -1)
+ if (rc != -1 && !S_ISREG(st->st_mode)) {
+ if (S_ISDIR(st->st_mode) &&
+ unlinkat(p->rootfd, f->path, AT_REMOVEDIR) == -1) {
+ ERR("%s: unlinkat", f->path);
+ return -1;
+ }
return 1;
-
- /*
-* Check if the file is already up to date, in which case close it
-* and go to the next one.
-*/
-
- if (fstat(*filefd, st) == -1) {
- ERR("%s: fstat", f->path);
- close(*filefd);
- *filefd = -1;
- return -1;
- } else if (!S_ISREG(st->st_mode)) {
- ERRX("%s: not regular", f->path);
- close(*filefd);
- *filefd = -1;
- return -1;
}
if (st->st_size == f->st.size &&
st->st_mtime == f->st.mtime) {
LOG3("%s: skipping: up to date", f->path);
- if (!rsync_set_metadata(sess, 0, *filefd, f, f->path)) {
+ if (!rsync_set_metadata_at(sess, 0, p->rootfd, f, f->path)) {
ERRX1("rsync_set_metadata");
- close(*filefd);
- *filefd = -1;
return -1;
}
- close(*filefd);
- *filefd = -1;
return 0;
}
+ *filefd = openat(p->rootfd, f->path, O_RDONLY | O_NOFOLLOW, 0);
+ if (*filefd == -1 && errno != ENOENT) {
+ ERR("%s: openat", f->path);
+ return -1;
+ }
+
/* file needs attention */
return 1;
}
@@ -785,8 +778,7 @@ rsync_uploader(struct upload *u, int *fi
off_t offs;
int c;
- /* This should never get called. */
-
+ /* Once finished this should never get called again. */
assert(u->state != UPLOAD_FINISHED);
/*
Re: citrus_none: don't allow codepoints > 0x7f
ping
On Sun, 2021-04-18 at 16:44 +0200, Martijn van Duren wrote:
> Hello tech@,
>
> Currently we accept codepoints 0-0xff in citrus_none, but the C/POSIX
> locale use 7 bit ascii, allowing us currently to return unspecified
> values. From code-inspection, NetBSD does the same thing we do (since
> they also use citrus, that's to be expected), but FreeBSD (from code-
> inspection) and glibc (from testing) return EILSEQ on >0x7f.
>
> Changing this also allows us to drop a few (now useless) casts and a
> wrapper-function.
>
> This change will make the CSI case for regress/lib/libc/locale/wcrtomb
> fail, but that's easy enough to fix.
>
> OK (after unlock)?
>
> martijn@
>
> Index: citrus_none.c
> ===
> RCS file: /cvs/src/lib/libc/citrus/citrus_none.c,v
> retrieving revision 1.9
> diff -u -p -r1.9 citrus_none.c
> --- citrus_none.c 7 Sep 2016 17:10:43 - 1.9
> +++ citrus_none.c 18 Apr 2021 14:40:59 -
> @@ -35,22 +35,6 @@
>
> #include "citrus_ctype.h"
>
> -static char wrapv(unsigned char);
> -
> -
> -/*
> - * Convert an unsigned char value into a char value without relying on
> - * signed overflow behavior.
> - */
> -static char
> -wrapv(unsigned char ch)
> -{
> - if (ch >= 0x80)
> - return (int)ch - 0x100;
> - else
> - return ch;
> -}
> -
> size_t
> _citrus_none_ctype_mbrtowc(wchar_t * __restrict pwc,
> const char * __restrict s, size_t n)
> @@ -59,8 +43,12 @@ _citrus_none_ctype_mbrtowc(wchar_t * __r
> return 0;
> if (n == 0)
> return -2;
> + if (*s > 0x7f) {
> + errno = EILSEQ;
> + return -1;
> + }
> if (pwc != NULL)
> - *pwc = (wchar_t)(unsigned char)*s;
> + *pwc = (wchar_t)*s;
> return *s != '\0';
> }
>
> @@ -74,7 +62,11 @@ _citrus_none_ctype_mbsnrtowcs(wchar_t *
> return strnlen(*src, nmc);
>
> for (i = 0; i < nmc && i < len; i++)
> - if ((dst[i] = (wchar_t)(unsigned char)(*src)[i]) == L'\0') {
> + if ((*src)[i] > 0x7f) {
> + errno = EILSEQ;
> + return -1;
> + }
> + if ((dst[i] = (wchar_t)(*src)[i]) == L'\0') {
> *src = NULL;
> return i;
> }
> @@ -89,12 +81,12 @@ _citrus_none_ctype_wcrtomb(char * __rest
> if (s == NULL)
> return 1;
>
> - if (wc < 0 || wc > 0xff) {
> + if (wc < 0 || wc > 0x7f) {
> errno = EILSEQ;
> return -1;
> }
>
> - *s = wrapv(wc);
> + *s = (char)wc;
> return 1;
> }
>
> @@ -107,7 +99,7 @@ _citrus_none_ctype_wcsnrtombs(char * __r
> if (dst == NULL) {
> for (i = 0; i < nwc; i++) {
> wchar_t wc = (*src)[i];
> - if (wc < 0 || wc > 0xff) {
> + if (wc < 0 || wc > 0x7f) {
> errno = EILSEQ;
> return -1;
> }
> @@ -119,12 +111,12 @@ _citrus_none_ctype_wcsnrtombs(char * __r
>
> for (i = 0; i < nwc && i < len; i++) {
> wchar_t wc = (*src)[i];
> - if (wc < 0 || wc > 0xff) {
> + if (wc < 0 || wc > 0x7f) {
> *src += i;
> errno = EILSEQ;
> return -1;
> }
> - dst[i] = wrapv(wc);
> + dst[i] = (char)wc;
> if (wc == L'\0') {
> *src = NULL;
> return i;
>
>
Re: printf(1): support \u and \U
On Fri, 07 May 2021 16:39:34 +0200, Martijn van Duren wrote: > Any takers? > > Updated diff after previous commit below. OK millert@ - todd
Re: printf(1): support \u and \U
Any takers?
Updated diff after previous commit below.
martijn@
On Sun, 2021-04-18 at 23:54 +0200, Martijn van Duren wrote:
> On Sun, 2021-04-18 at 22:53 +0200, Martijn van Duren wrote:
> > On Sun, 2021-04-18 at 17:52 +0200, Martijn van Duren wrote:
> > > I'm always frustrated when a unicode character question comes up and I
> > > have to look up the UTF-8 byte sequence to reproduce it. When fixing \x
> > > I found the \u and \U escape sequences in gprintf, which seem mighty
> > > handy for this exact case.
> > >
> > > My implementation differs from gprintf in that leading zeroes can be
> > > omitted, but I kept \u and \U for both compatability and for cases like
> > > \u00ebb, where I don't want to add 6 zeroes just to get my desired
> > > unicode character in front of an isxdigit(3) character.
> > >
> > > gprintf talks about "Unicode (ISO/IEC 10646)" in their manpage for the
> > > \u case and just Unicode for the \U case. I read that glibc uses 10646
> > > internally for wchar_t, but I have no idea how 10646 might differ from
> > > true unicode for >= 0 <= 0x, so I stuck with just the term unicode
> > > in the manpage part.
> > >
> > > gprintf prints the \u or \U form for characters > 0x7f and < 0x100 in
> > > the C locale, where this diff currently outputs these byte values.
> > > My previous diff[0] should fix this.
> > >
> > > OK after unlock?
> > >
> > > martijn@
> > >
> > > [0] https://marc.info/?l=openbsd-tech&m=161875718324367&w=2
> > >
> > Guenther asked me offlist to not ignore leading zeroes.
> > Our printf implementation is already lenient, so the diff below still
> > allows, but throws a warning and returns an error code on exit, similar
> > to other violations.
> >
> And of course the [1-3] and [1-7] in the manpage need to be scratched.
> And I had an of by 1 in the counter...
>
Index: printf.1
===
RCS file: /cvs/src/usr.bin/printf/printf.1,v
retrieving revision 1.35
diff -u -p -r1.35 printf.1
--- printf.17 May 2021 14:31:27 - 1.35
+++ printf.17 May 2021 14:39:07 -
@@ -108,6 +108,14 @@ Write an 8-bit character whose ASCII val
the 1- or 2-digit hexadecimal
number
.Ar num .
+.It Cm \eu Ns Ar num
+Write a unicode character whose value is
+the 4-digit hexadecimal number
+.Ar num .
+.It Cm \eU Ns Ar num
+Write a unicode character whose value is
+the 8-digit hexadecimal number
+.Ar num .
.El
.Pp
Each format specification is introduced by the percent
@@ -361,6 +369,19 @@ no argument is used.
In no case does a non-existent or small field width cause truncation of
a field; padding takes place only if the specified field width exceeds
the actual width.
+.Sh ENVIRONMENT
+.Bl -tag -width LC_CTYPE
+.It Ev LC_CTYPE
+The character encoding
+.Xr locale 1 .
+It decides which unicode values can be output in the current character
encoding.
+If a character can't be displayed in the current locale it falls back to the
+shortest full
+.Cm \eu Ns Ar num
+or
+.Cm \eU Ns Ar num
+presentation.
+.El
.Sh EXIT STATUS
.Ex -std printf
.Sh EXAMPLES
@@ -389,6 +410,8 @@ were set.
.Pp
The escape sequences
.Cm \ee ,
+.Cm \eu ,
+.Cm \eU ,
.Cm \ex
and
.Cm \e' ,
Index: printf.c
===
RCS file: /cvs/src/usr.bin/printf/printf.c,v
retrieving revision 1.27
diff -u -p -r1.27 printf.c
--- printf.c7 May 2021 14:31:27 - 1.27
+++ printf.c7 May 2021 14:39:07 -
@@ -33,10 +33,12 @@
#include
#include
#include
+#include
#include
#include
#include
#include
+#include
static int print_escape_str(const char *);
static int print_escape(const char *);
@@ -79,6 +81,8 @@ main(int argc, char *argv[])
char convch, nextch;
char *format;
+ setlocale(LC_CTYPE, "");
+
if (pledge("stdio", NULL) == -1)
err(1, "pledge");
@@ -275,8 +279,10 @@ static int
print_escape(const char *str)
{
const char *start = str;
+ char mbc[MB_LEN_MAX + 1];
+ wchar_t wc = 0;
int value = 0;
- int c;
+ int c, i;
str++;
@@ -344,6 +350,29 @@ print_escape(const char *str)
case 't': /* tab */
putchar('\t');
break;
+
+ case 'U':
+ case 'u':
+ c = *str == 'U' ? 8 : 4;
+ str++;
+ for (; c-- && isxdigit((unsigned char)*str); str++) {
+ wc <<= 4;
+ wc += hextobin(*str);
+ }
+ if (c != -1) {
+ warnx("missing hexadecimal number in escape");
+ rval = 1;
+ }
+ if ((c = wctomb(mbc, wc)) == -1) {
+ printf("\\%c%0*X", wc > 0x ? 'U' : 'u',
+ wc > 0x ? 8 : 4, wc);
+ wc = L'\0';
+ wctomb(NULL, wc);
+ }
snmpd rename context to pdutype
When moving the traphandler to the snmpe process I overlooked the fact
that "type" is being saved inside the switch statement under the
sm_context name. RFC3411 talks about pduType, and the name context means
something completely different in the v3 world.
The diff below moves our naming closer to the RFCs, which should
hopefully prevent further confusion in the future.
While here I made the debug output print the pduType in a human readable
format.
The invalid varbind check can be simplified a simple "{}" in the
ober_scanf_elements allowing me to just drop the type variable.
OK?
martijn@
Index: snmp.h
===
RCS file: /cvs/src/usr.sbin/snmpd/snmp.h,v
retrieving revision 1.16
diff -u -p -r1.16 snmp.h
--- snmp.h 30 Jun 2020 17:11:49 - 1.16
+++ snmp.h 7 May 2021 14:17:12 -
@@ -77,7 +77,7 @@ enum snmp_version {
SNMP_V3 = 3
};
-enum snmp_context {
+enum snmp_pdutype {
SNMP_C_GETREQ = 0,
SNMP_C_GETNEXTREQ = 1,
SNMP_C_GETRESP = 2,
Index: snmpd.h
===
RCS file: /cvs/src/usr.sbin/snmpd/snmpd.h,v
retrieving revision 1.94
diff -u -p -r1.94 snmpd.h
--- snmpd.h 5 Feb 2021 10:30:45 - 1.94
+++ snmpd.h 7 May 2021 14:17:12 -
@@ -384,7 +384,7 @@ struct snmp_message {
socklen_tsm_slen;
int sm_sock_tcp;
int sm_aflags;
- int sm_type;
+ enum snmp_pdutypesm_pdutype;
struct event sm_sockev;
char sm_host[HOST_NAME_MAX+1];
in_port_tsm_port;
@@ -405,7 +405,6 @@ struct snmp_message {
/* V1, V2c */
char sm_community[SNMPD_MAXCOMMUNITYLEN];
- int sm_context;
/* V3 */
long longsm_msgid;
Index: snmpe.c
===
RCS file: /cvs/src/usr.sbin/snmpd/snmpe.c,v
retrieving revision 1.70
diff -u -p -r1.70 snmpe.c
--- snmpe.c 22 Feb 2021 11:31:09 - 1.70
+++ snmpe.c 7 May 2021 14:17:12 -
@@ -41,6 +41,7 @@
#include "mib.h"
voidsnmpe_init(struct privsep *, struct privsep_proc *, void *);
+const char *snmpe_pdutype2string(enum snmp_pdutype);
int snmpe_parse(struct snmp_message *);
voidsnmpe_tryparse(int, struct snmp_message *);
int snmpe_parsevarbinds(struct snmp_message *);
@@ -194,6 +195,36 @@ snmpe_bind(struct address *addr)
return (-1);
}
+const char *
+snmpe_pdutype2string(enum snmp_pdutype pdutype)
+{
+ static char unknown[sizeof("Unknown (4294967295)")];
+
+ switch (pdutype) {
+ case SNMP_C_GETREQ:
+ return "GetRequest";
+ case SNMP_C_GETNEXTREQ:
+ return "GetNextRequest";
+ case SNMP_C_GETRESP:
+ return "Response";
+ case SNMP_C_SETREQ:
+ return "SetRequest";
+ case SNMP_C_TRAP:
+ return "Trap";
+ case SNMP_C_GETBULKREQ:
+ return "GetBulkRequest";
+ case SNMP_C_INFORMREQ:
+ return "InformRequest";
+ case SNMP_C_TRAPV2:
+ return "SNMPv2-Trap";
+ case SNMP_C_REPORT:
+ return "Report";
+ }
+
+ snprintf(unknown, sizeof(unknown), "Unknown (%u)", pdutype);
+ return unknown;
+}
+
int
snmpe_parse(struct snmp_message *msg)
{
@@ -202,7 +233,6 @@ snmpe_parse(struct snmp_message *msg)
struct ber_element *a;
long longver, req;
long longerrval, erridx;
- unsigned int type;
u_intclass;
char*comn;
char*flagstr, *ctxname;
@@ -271,15 +301,15 @@ snmpe_parse(struct snmp_message *msg)
goto fail;
}
- if (ober_scanf_elements(msg->sm_pdu, "t{e", &class, &type, &a) != 0)
+ if (ober_scanf_elements(msg->sm_pdu, "t{e", &class, &(msg->sm_pdutype),
+ &a) != 0)
goto parsefail;
/* SNMP PDU context */
if (class != BER_CLASS_CONTEXT)
goto parsefail;
- msg->sm_type = type;
- switch (type) {
+ switch (msg->sm_pdutype) {
case SNMP_C_GETBULKREQ:
if (msg->sm_version == SNMP_V1) {
stats->snmp_inbadversions++;
@@ -294,7 +324,7 @@ snmpe_parse(struct snmp_message *msg)
/* FALLTHROUGH */
case SNMP_C_GETNEXTREQ:
- if (type == SNMP_C_GETNEXTREQ)
+ if (msg->sm_pdutype == SNMP_C_GETNEXTREQ)
stats->snmp_ingetnexts++;
if (!(msg->sm_aflags & ADDRESS_FLAG_READ)) {
msg->sm_errstr = "read requests disa
Re: Time to commit FUSE changes
On 2021/05/07 17:14, Helg wrote: > Hi tech@ > > Now that 6.9 has been released I'd like to commit my changes to replace > libfuse from base with the reference implementation from ports. > > Can I please have an OK? I'll commit the ports changes at the same time > (separate email already sent to ports@). If you would like more info > please let me know. I've submitted this before and there are already a > few of you that are aware and have been a great help to me. > > The main changes are to fuse_device.c and fusebuf.h where the protocol > is implemented. The other big change of course is to delete all of libfuse. I am OK with those though I have only tested things and not read over the src diff particularly thoroughly. It involves a base library moving to ports and machines with new kernels using FUSE-based software will need updated packages to work. I think this is not a problem to commit now as A) port build machines for the more common arch have moved past release and B) only a handful of ports link to this at all and even fewer of those *really* use it (there are a number of ports which are used for another purpose which also have FUSE support but that's not their main focus).
rsync exit code and error cleanup
Currently our rsync does not follow the exit codes from rsync. Also the
error handling is complex because ERR() and ERRX() are not terminating the
process.
This diff tries to start cleaning up the mess a bit. Introduce some exit
codes to use and apply them in places where it is obvious.
The rsync server process should normally communicate errors back via the
rsync socket but there are some errors where this will most probably fail
as well. A good example are memory failures.
I used ERR_IPC as a kind of catchall for any system error that pops up
(fork, socketpair but also pledge, unveil).
The goal is to reduce the amount of ERR(), ERRX() and ERRX1() from rsync.
This should simplify the code base.
I also synced the mkpath() call with the one from mkdir and handle the
error now outside of the call. Again I think the result is cleaner.
OK?
--
:wq Claudio
Index: client.c
===
RCS file: /cvs/src/usr.bin/rsync/client.c,v
retrieving revision 1.15
diff -u -p -r1.15 client.c
--- client.c8 May 2019 20:00:25 - 1.15
+++ client.c7 May 2021 08:29:10 -
@@ -43,7 +43,7 @@ rsync_client(const struct opts *opts, in
if (pledge("stdio unix rpath wpath cpath dpath fattr chown getpw
unveil",
NULL) == -1)
- err(1, "pledge");
+ err(ERR_IPC, "pledge");
memset(&sess, 0, sizeof(struct sess));
sess.opts = opts;
Index: extern.h
===
RCS file: /cvs/src/usr.bin/rsync/extern.h,v
retrieving revision 1.36
diff -u -p -r1.36 extern.h
--- extern.h31 Mar 2021 19:45:16 - 1.36
+++ extern.h7 May 2021 08:22:32 -
@@ -42,6 +42,19 @@
#defineCSUM_LENGTH_PHASE2 (16)
/*
+ * Rsync error codes.
+ */
+#define ERR_SYNTAX 1
+#define ERR_PROTOCOL 2
+#define ERR_SOCK_IO10
+#define ERR_FILE_IO11
+#define ERR_WIREPROTO 12
+#define ERR_IPC14 /* catchall for any kind of syscall
error */
+#define ERR_TERMIMATED 16
+#define ERR_WAITPID21
+#define ERR_NOMEM 22
+
+/*
* Use this for --timeout.
* All poll events will use it and catch time-outs.
*/
Index: fargs.c
===
RCS file: /cvs/src/usr.bin/rsync/fargs.c,v
retrieving revision 1.17
diff -u -p -r1.17 fargs.c
--- fargs.c 8 May 2019 20:00:25 - 1.17
+++ fargs.c 7 May 2021 08:19:29 -
@@ -17,6 +17,7 @@
#include
#include
+#include
#include
#include
#include
@@ -51,7 +52,7 @@ fargs_cmdline(struct sess *sess, const s
if (sess->opts->ssh_prog) {
ap = strdup(sess->opts->ssh_prog);
if (ap == NULL)
- goto out;
+ err(ERR_NOMEM, NULL);
while ((arg = strsep(&ap, " \t")) != NULL) {
if (arg[0] == '\0') {
@@ -127,8 +128,4 @@ fargs_cmdline(struct sess *sess, const s
addargs(&args, "%s", f->sink);
return args.list;
-out:
- freeargs(&args);
- ERR("calloc");
- return NULL;
}
Index: main.c
===
RCS file: /cvs/src/usr.bin/rsync/main.c,v
retrieving revision 1.53
diff -u -p -r1.53 main.c
--- main.c 31 Mar 2021 19:45:16 - 1.53
+++ main.c 7 May 2021 08:30:18 -
@@ -83,18 +83,18 @@ fargs_parse(size_t argc, char *argv[], s
/* Allocations. */
if ((f = calloc(1, sizeof(struct fargs))) == NULL)
- err(1, "calloc");
+ err(ERR_NOMEM, NULL);
f->sourcesz = argc - 1;
if ((f->sources = calloc(f->sourcesz, sizeof(char *))) == NULL)
- err(1, "calloc");
+ err(ERR_NOMEM, NULL);
for (i = 0; i < argc - 1; i++)
if ((f->sources[i] = strdup(argv[i])) == NULL)
- err(1, "strdup");
+ err(ERR_NOMEM, NULL);
if ((f->sink = strdup(argv[i])) == NULL)
- err(1, "strdup");
+ err(ERR_NOMEM, NULL);
/*
* Test files for its locality.
@@ -109,15 +109,16 @@ fargs_parse(size_t argc, char *argv[], s
if (fargs_is_remote(f->sink)) {
f->mode = FARGS_SENDER;
if ((f->host = strdup(f->sink)) == NULL)
- err(1, "strdup");
+ err(ERR_NOMEM, NULL);
}
if (fargs_is_remote(f->sources[0])) {
if (f->host != NULL)
- errx(1, "both source and destination cannot be remote
files");
+ errx(ERR_SYNTAX, "both source and destination "
+ "cannot be remote files");
f->mode = FARGS_RECEIVER;
if ((f->host = strdup(f->sources[0])) == NULL)
- err(1, "strdup");
Re: macppc: add ld.script for kernel, ofwboot
> Date: Fri, 7 May 2021 00:04:57 -0400
> From: George Koehler
>
> Hello tech list,
>
> I want macppc to switch from ld.bfd to ld.lld, but there is a problem
> when lld links ofwboot or the kernel. I propose to fix it by adding
> ld.script for both. These scripts also work with ld.bfd, so I want to
> commit my diff at the end of this mail, ok?
>
> lld sets the symbol "etext" to a nonsense value like 0x1034. In
> ofwboot, wrong "etext" causes freeze, failure to boot kernel. (Wrong
> "etext" in kernel doesn't cause an obvious problem.) Other lld arches
> use an ld.script to set a correct "etext" in kernel.
>
> I copied the ld.script from powerpc64's kernel and made these changes
> for macppc's kernel:
> - change "_start" to "start" to match macppc/locore0.S
> - remove PT_DYNAMIC and sections (don't exist in macppc kernel)
> - put .text at 0x00100114 to match Makefile
> - remove symbols like "_erodata" (not used by macppc kernel)
> - add ".rodata.*" and such, so sections and segments look correct
Makes sense to me. It seems ldd always seems to require a little bit
more coercion to produce non-standard binaries. We use linker scripts
for the various EFI bootloaders as well.
ok kettenis@
> | --- arch/powerpc64/conf/ld.script Sat Jul 18 09:16:32 2020
> | +++ arch/macppc/conf/ld.script Sat Apr 24 11:52:34 2021
> | @@ -16,18 +16,17 @@
> | * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> | */
> |
> | -ENTRY(_start)
> | +ENTRY(start)
> |
> | PHDRS
> | {
> | text PT_LOAD;
> | - dynamic PT_DYNAMIC;
> | openbsd_randomize PT_OPENBSD_RANDOMIZE;
> | }
> |
> | SECTIONS
> | {
> | - . = 0x0010;
> | + . = 0x00100114;
> | .text :
> | {
> | *(.text)
> | @@ -35,49 +34,35 @@
> | PROVIDE (etext = .);
> | PROVIDE (_etext = .);
> |
> | - . = ALIGN(4096);
> | - .rela.dyn : { *(.rela.dyn) }
> | -
> | - .dynamic :
> | + .rodata :
> | {
> | - *(.dynamic)
> | - } :dynamic :text
> | + *(.rodata .rodata.*)
> | + } :text
> |
> | - .rodata :
> | + .data.rel.ro :
> | {
> | - *(.rodata)
> | *(.data.rel.ro)
> | } :text
> |
> | .openbsd.randomdata :
> | {
> | - *(.openbsd.randomdata)
> | + *(.openbsd.randomdata .openbsd.randomdata.*)
> | } :openbsd_randomize :text
> | - PROVIDE (_erodata = .);
> |
> | - . = ALIGN(4096);
> | .data :
> | {
> | *(.data)
> | } :text
> |
> | - . = ALIGN(4096);
> | - .got : { *(.got) }
> | - .toc : { *(.toc) }
> | + .sbss :
> | + {
> | + *(.sbss)
> | + }
> |
> | - PROVIDE (__bss_start = .);
> | .bss :
> | {
> | *(.bss)
> | }
> | PROVIDE (end = .);
> | PROVIDE (_end = .);
> | -
> | - /DISCARD/ :
> | - {
> | - *(.dynsym)
> | - *(.dynstr)
> | - *(.gnu.hash)
> | - *(.hash)
> | - }
> | }
>
> Then I made these changes for ofwboot:
> - use "_start" and 0x0002 to match Makefile
> - remove randomdata (doesn't exist in ofwboot)
>
> | --- arch/macppc/conf/ld.script Sat Apr 24 11:52:34 2021
> | +++ arch/macppc/stand/ofwboot/ld.script Sat Apr 24 11:52:34 2021
> | @@ -16,17 +16,17 @@
> | * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> | */
> |
> | -ENTRY(start)
> | +ENTRY(_start)
> |
> | PHDRS
> | {
> | text PT_LOAD;
> | - openbsd_randomize PT_OPENBSD_RANDOMIZE;
> | }
> |
> | SECTIONS
> | {
> | - . = 0x00100114;
> | + /* Must match RELOC in Makefile */
> | + . = 0x0002;
> | .text :
> | {
> | *(.text)
> | @@ -43,11 +43,6 @@
> | {
> | *(.data.rel.ro)
> | } :text
> | -
> | - .openbsd.randomdata :
> | - {
> | - *(.openbsd.randomdata .openbsd.randomdata.*)
> | - } :openbsd_randomize :text
> |
> | .data :
> | {
>
> Index: arch/macppc/conf/Makefile.macppc
> ===
> RCS file: /cvs/src/sys/arch/macppc/conf/Makefile.macppc,v
> retrieving revision 1.101
> diff -u -p -r1.101 Makefile.macppc
> --- arch/macppc/conf/Makefile.macppc 28 Jan 2021 17:39:03 - 1.101
> +++ arch/macppc/conf/Makefile.macppc 6 May 2021 20:01:08 -
> @@ -51,7 +51,7 @@ DEBUG?= -g
> COPTIMIZE?= -O2
> CFLAGS= ${DEBUG} ${CWARNFLAGS} ${CMACHFLAGS} ${COPTIMIZE}
> ${COPTS} ${PIPE}
> AFLAGS= -D_LOCORE ${CMACHFLAGS}
> -LINKFLAGS= -N -Ttext 100114 -e start --warn-common -nopie
> +LINKFLAGS= -N -T ld.script --warn-common -nopie
>
> .if ${MACHINE} == "powerpc64"
> CFLAGS+= -m32
> Index: arch/macppc/conf/ld.script
> ===
> RCS file: /cvs/src/sys/arch/macppc/conf/ld.script,v
> retrieving revision 1.1
> diff -u -p -r1.1 ld.script
> --- arch/macppc/conf/ld.script13 Jun 2017 01:42:52 - 1.1
> +++ arch/macp
