From: Rich Felker <dalias@libc.org>
To: musl@lists.openwall.com
Subject: Re: [PATCH 0/9] linux v4.16 update
Date: Sat, 9 Jun 2018 20:51:54 -0400 [thread overview]
Message-ID: <20180610005154.GF1392@brightrain.aerifal.cx> (raw)
In-Reply-To: <20180609224158.GQ4418@port70.net>
On Sun, Jun 10, 2018 at 12:41:59AM +0200, Szabolcs Nagy wrote:
> * Rich Felker <dalias@libc.org> [2018-05-15 13:39:23 -0400]:
> > On Sat, Apr 28, 2018 at 09:56:56PM +0200, Szabolcs Nagy wrote:
> > > From f7eb8934396890e39b9e5e2b4fdd1534f1c024cc Mon Sep 17 00:00:00 2001
> > > From: Szabolcs Nagy <nsz@port70.net>
> > > Date: Mon, 16 Apr 2018 22:16:29 +0000
> > > Subject: [PATCH 1/9] siginfo struct change following linux v4.16
> > >
> > > this is supposed to be a cosmetic change only, not affecting api or abi,
> > > follows linux commit b68a68d3dcc15ebbf23cbe91af1abf57591bd96b and
> > > 859d880cf544dbe095ce97534ef04cd88ba2f2b4 with slightly different field
> > > names to follow musl conventions (which must be different to avoid
> > > depending on anonymous union support and polluting the namespace).
> > > ---
> > > include/signal.h | 12 ++++++++----
> > > 1 file changed, 8 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/include/signal.h b/include/signal.h
> > > index a4f85cca..d68331e8 100644
> > > --- a/include/signal.h
> > > +++ b/include/signal.h
> > > @@ -123,13 +123,17 @@ typedef struct {
> > > } __si_common;
> > > struct {
> > > void *si_addr;
> > > - short si_addr_lsb;
> > > union {
> > > + short si_addr_lsb;
> > > struct {
> > > + void *__dummy_bnd;
> > > void *si_lower;
> > > void *si_upper;
> > > } __addr_bnd;
> > > - unsigned si_pkey;
> > > + struct {
> > > + void *__dummy_pkey;
> > > + unsigned si_pkey;
> > > + } __addr_pkey;
> > > } __first;
> > > } __sigfault;
> > > struct {
> > > @@ -150,10 +154,10 @@ typedef struct {
> > > #define si_stime __si_fields.__si_common.__second.__sigchld.si_stime
> > > #define si_value __si_fields.__si_common.__second.si_value
> > > #define si_addr __si_fields.__sigfault.si_addr
> > > -#define si_addr_lsb __si_fields.__sigfault.si_addr_lsb
> > > +#define si_addr_lsb __si_fields.__sigfault.__first.si_addr_lsb
> > > #define si_lower __si_fields.__sigfault.__first.__addr_bnd.si_lower
> > > #define si_upper __si_fields.__sigfault.__first.__addr_bnd.si_upper
> > > -#define si_pkey __si_fields.__sigfault.__first.si_pkey
> > > +#define si_pkey __si_fields.__sigfault.__first.__addr_pkey.si_pkey
> > > #define si_band __si_fields.__sigpoll.si_band
> > > #define si_fd __si_fields.__sigpoll.si_fd
> > > #define si_timerid __si_fields.__si_common.__first.__timer.si_timerid
> >
> > Is there any benefit to making this change?
> >
>
> only to follow the linux headers might make it easier to track changes
>
> turns out the change was wrong though and now it's even more messy:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=8420f71943ae96dcd78da5bd4a5c2827419d340c
It looks like the change is just for archs where pointers aren't
aligned, of which we presently don't have any, but it makes the whole
change rather blah, and I don't like the new version either. If they
mean to reserve space for a short plus any alignment it causes, they
should just add a short there rather than a wacky-sized char array.
Anyway my leaning is just to drop this unless it turns out that future
additions to the struct depend on it, and we can address that if/when
it happens.
> > > From de4ca3284d759563322c795479601e9eee394832 Mon Sep 17 00:00:00 2001
> > > From: Szabolcs Nagy <nsz@port70.net>
> > > Date: Sat, 28 Apr 2018 17:25:41 +0000
> > > Subject: [PATCH 9/9] [RFC] add new memory mapping related apis
> > >
> > > memfd_create (linux v3.17)
> > > mlock2 (linux v4.4)
> > > pkey_alloc (linux v4.9)
> > > pkey_free (linux v4.9)
> > > pkey_mprotect (linux v4.9)
> > > pkey_get (glibc 2.27)
> > > pkey_set (glibc 2.27)
> > >
> > > notes:
> > >
> > > - pkey_alloc type is inconsistent between the manual and
> > > the glibc source (unsigned int vs unsigned long args)
> >
> > This needs to be resolved, I think.
> >
>
> glibc is right, the man was documenting the syscall layer
OK.
> > > - pkey_get / pkey_set are not syscalls (query/update the
> > > pkru register on x86), not implemented yet.
> > > - fallback in mlock2 to mlock and pkey_mprotect to mprotect
> > > follows glibc.
> > > - mlock2 EINVAL return means invalid (or unsupported) flags.
> > > - moved MLOCK_ONFAULT under _GNU_SOURCE following glibc.
> > > ---
> > > arch/powerpc/bits/mman.h | 4 ++++
> > > arch/powerpc64/bits/mman.h | 4 ++++
> > > include/sys/mman.h | 24 +++++++++++++++++++++---
> > > src/linux/memfd_create.c | 10 ++++++++++
> > > src/linux/mlock2.c | 15 +++++++++++++++
> > > src/linux/pkey_alloc.c | 15 +++++++++++++++
> > > src/linux/pkey_get.c | 9 +++++++++
> > > src/linux/pkey_mprotect.c | 15 +++++++++++++++
> > > src/linux/pkey_set.c | 9 +++++++++
> > > 9 files changed, 102 insertions(+), 3 deletions(-)
> >
> > Am I correct in assuming the header & external function interfaces
> > match how glibc is exposing these functions?
> >
> > One thing that looks a little bit weird to me is conditioning
> > existence of some functions on the syscall macro:
> >
> > > diff --git a/src/linux/pkey_alloc.c b/src/linux/pkey_alloc.c
> > > new file mode 100644
> > > index 00000000..0b0770a6
> > > --- /dev/null
> > > +++ b/src/linux/pkey_alloc.c
> > > @@ -0,0 +1,15 @@
> > > +#include "syscall.h"
> > > +
> > > +#ifdef SYS_pkey_alloc
> > > +#define _GNU_SOURCE 1
> > > +#include <sys/mman.h>
> > > +int pkey_alloc(unsigned flags, unsigned access)
> > > +{
> > > + return syscall(SYS_pkey_alloc, flags, access);
> > > +}
> > > +
> > > +int pkey_free(int pkey)
> > > +{
> > > + return syscall(SYS_pkey_free, pkey);
> > > +}
> > > +#endif
> >
> > where others fail with ENOSYS when the arch has no definition. I think
> > it's also problematic that syscall.h is being included before #define
> > _GNU_SOURCE. This would break under changes in how the features.h
> > mechanisms work.
> >
>
> ok this should fail ENOSYS when not available.
OK.
Rich
prev parent reply other threads:[~2018-06-10 0:51 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-04-28 19:56 Szabolcs Nagy
2018-04-28 21:39 ` Szabolcs Nagy
2018-05-15 17:39 ` Rich Felker
2018-06-09 22:41 ` Szabolcs Nagy
2018-06-10 0:51 ` Rich Felker [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180610005154.GF1392@brightrain.aerifal.cx \
--to=dalias@libc.org \
--cc=musl@lists.openwall.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.vuxu.org/mirror/musl/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).