* [PATCH 2/2] clock_nanosleep.2, nanosleep.2: Use 'duration' rather than 'request'
From: Alejandro Colomar @ 2024-03-03 12:15 UTC (permalink / raw)
To: Elliott Hughes, Stefan Puiu, Bruno Haible, linux-man
Cc: Alejandro Colomar, GNU C Library, linux-api
In-Reply-To: <ZUIlirG-ypudgpbK@debian>
[-- Attachment #1: Type: text/plain, Size: 4372 bytes --]
It seems much more clear.
Suggested-by: Elliott Hughes <enh@google.com>
Cc: Stefan Puiu <stefan.puiu@gmail.com>
Cc: Bruno Haible <bruno@clisp.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
---
man2/clock_nanosleep.2 | 20 ++++++++++----------
man2/nanosleep.2 | 12 ++++++------
2 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/man2/clock_nanosleep.2 b/man2/clock_nanosleep.2
index 5bda50e18..0eedc1277 100644
--- a/man2/clock_nanosleep.2
+++ b/man2/clock_nanosleep.2
@@ -19,7 +19,7 @@ .SH SYNOPSIS
.nf
.P
.BI "int clock_nanosleep(clockid_t " clockid ", int " flags ,
-.BI " const struct timespec *" request ,
+.BI " const struct timespec *" duration ,
.BI " struct timespec *_Nullable " remain );
.fi
.P
@@ -94,7 +94,7 @@ .SH DESCRIPTION
If
.I flags
is 0, then the value specified in
-.I request
+.I duration
is interpreted as an interval relative to the current
value of the clock specified by
.IR clockid .
@@ -104,11 +104,11 @@ .SH DESCRIPTION
is
.BR TIMER_ABSTIME ,
then
-.I request
+.I duration
is interpreted as an absolute time as measured by the clock,
.IR clockid .
If
-.I request
+.I duration
is less than or equal to the current value of the clock,
then
.BR clock_nanosleep ()
@@ -117,7 +117,7 @@ .SH DESCRIPTION
.BR clock_nanosleep ()
suspends the execution of the calling thread
until either at least the time specified by
-.I request
+.I duration
has elapsed,
or a signal is delivered that causes a signal handler to be called or
that terminates the process.
@@ -138,7 +138,7 @@ .SH DESCRIPTION
.BR clock_nanosleep ()
again and complete a (relative) sleep.
.SH RETURN VALUE
-On successfully sleeping for the requested interval,
+On successfully sleeping for the requested duration,
.BR clock_nanosleep ()
returns 0.
If the call is interrupted by a signal handler or encounters an error,
@@ -146,7 +146,7 @@ .SH RETURN VALUE
.SH ERRORS
.TP
.B EFAULT
-.I request
+.I duration
or
.I remain
specified an invalid address.
@@ -179,8 +179,8 @@ .SH HISTORY
Linux 2.6,
glibc 2.1.
.SH NOTES
-If the interval specified in
-.I request
+If the
+.I duration
is not an exact multiple of the granularity underlying clock (see
.BR time (7)),
then the interval will be rounded up to the next multiple.
@@ -216,7 +216,7 @@ .SH NOTES
is
.BR TIMER_ABSTIME .
(An absolute sleep can be restarted using the same
-.I request
+.I duration
argument.)
.P
POSIX.1 specifies that
diff --git a/man2/nanosleep.2 b/man2/nanosleep.2
index a8d9f5a8a..6272c21e6 100644
--- a/man2/nanosleep.2
+++ b/man2/nanosleep.2
@@ -22,7 +22,7 @@ .SH SYNOPSIS
.nf
.B #include <time.h>
.P
-.BI "int nanosleep(const struct timespec *" req ,
+.BI "int nanosleep(const struct timespec *" duration ,
.BI " struct timespec *_Nullable " rem );
.fi
.P
@@ -39,7 +39,7 @@ .SH DESCRIPTION
.BR nanosleep ()
suspends the execution of the calling thread
until either at least the time specified in
-.I *req
+.I *duration
has elapsed, or the delivery of a signal
that triggers the invocation of a handler in the calling thread or
that terminates the process.
@@ -80,7 +80,7 @@ .SH DESCRIPTION
and it makes the task of resuming a sleep that has been
interrupted by a signal handler easier.
.SH RETURN VALUE
-On successfully sleeping for the requested interval,
+On successfully sleeping for the requested duration,
.BR nanosleep ()
returns 0.
If the call is interrupted by a signal handler or encounters an error,
@@ -139,7 +139,7 @@ .SH VERSIONS
.BR nanosleep ()
function; ...
Consequently, these time services shall expire when the requested relative
-interval elapses, independently of the new or old value of the clock.
+duration elapses, independently of the new or old value of the clock.
.RE
.SH STANDARDS
POSIX.1-2008.
@@ -158,8 +158,8 @@ .SH HISTORY
This special extension was removed in Linux 2.5.39,
and is thus not available in Linux 2.6.0 and later kernels.
.SH NOTES
-If the interval specified in
-.I req
+If the
+.I duration
is not an exact multiple of the granularity underlying clock (see
.BR time (7)),
then the interval will be rounded up to the next multiple.
--
2.43.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* Re: [PATCH 2/2] clock_nanosleep.2, nanosleep.2: Use 'duration' rather than 'request'
From: Alejandro Colomar @ 2024-03-03 12:55 UTC (permalink / raw)
To: Bruno Haible
Cc: Elliott Hughes, Stefan Puiu, linux-man, GNU C Library, linux-api
In-Reply-To: <5882437.otsE0voPBg@nimes>
[-- Attachment #1: Type: text/plain, Size: 1574 bytes --]
Hi Bruno,
On Sun, Mar 03, 2024 at 01:45:37PM +0100, Bruno Haible wrote:
> Alejandro Colomar wrote:
> > man2/clock_nanosleep.2 | 20 ++++++++++----------
> > man2/nanosleep.2 | 12 ++++++------
>
> The change to nanosleep.2 seems mostly fine. Except that the
> term "requested relative duration" (line 142) raises questions;
> what about changing that to "requested duration"?
Yeah, I had doubts about that one. Probably I should drop 'relative'.
>
> The change to clock_nanosleep.2 seems wrong. There are two cases
> (quoting the old text):
>
> If flags is 0, then the value specified in request is interpreted
> as an interval relative to the current value of the clock
> specified by clockid.
>
> If flags is TIMER_ABSTIME, then request is interpreted as an
> absolute time as measured by the clock, clockid. If request is
> less than or equal to the current value of the clock, then
> clock_nanosleep() returns immediately without suspending the calling
> thread.
>
> In the first case, the argument is a duration. In the second case, the
> argument is an absolute time point; it would be wrong and very confusing
> to denote it as "duration".
Hmm, thanks! I guess we'll have to keep 'request' in clock_nanosleep(3)
unless someone comes up with a better name. Elliott, you may want to
partially revert that change in bionic.
Have a lovely day!
Alex
--
<https://www.alejandro-colomar.es/>
Looking for a remote C programming job at the moment.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH v2 2/2] nanosleep.2: Use 'duration' rather than 'request'
From: Alejandro Colomar @ 2024-03-03 13:02 UTC (permalink / raw)
To: Bruno Haible, linux-man, Elliott Hughes
Cc: Alejandro Colomar, Stefan Puiu, GNU C Library, linux-api
In-Reply-To: <ZeRzS6mENO8kOh1W@debian>
[-- Attachment #1: Type: text/plain, Size: 2348 bytes --]
It seems much more clear.
Suggested-by: Elliott Hughes <enh@google.com>
Cc: Stefan Puiu <stefan.puiu@gmail.com>
Cc: Bruno Haible <bruno@clisp.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
---
v2:
- 2/2:
- Drop changes to clock_nanosleep(2). [Bruno]
- Don't say "relative duration". [Bruno]
- 1/2: unchanged, so not resent.
man2/nanosleep.2 | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/man2/nanosleep.2 b/man2/nanosleep.2
index a8d9f5a8a..e7132ee32 100644
--- a/man2/nanosleep.2
+++ b/man2/nanosleep.2
@@ -22,7 +22,7 @@ .SH SYNOPSIS
.nf
.B #include <time.h>
.P
-.BI "int nanosleep(const struct timespec *" req ,
+.BI "int nanosleep(const struct timespec *" duration ,
.BI " struct timespec *_Nullable " rem );
.fi
.P
@@ -39,7 +39,7 @@ .SH DESCRIPTION
.BR nanosleep ()
suspends the execution of the calling thread
until either at least the time specified in
-.I *req
+.I *duration
has elapsed, or the delivery of a signal
that triggers the invocation of a handler in the calling thread or
that terminates the process.
@@ -80,7 +80,7 @@ .SH DESCRIPTION
and it makes the task of resuming a sleep that has been
interrupted by a signal handler easier.
.SH RETURN VALUE
-On successfully sleeping for the requested interval,
+On successfully sleeping for the requested duration,
.BR nanosleep ()
returns 0.
If the call is interrupted by a signal handler or encounters an error,
@@ -138,8 +138,9 @@ .SH VERSIONS
service based upon this clock, including the
.BR nanosleep ()
function; ...
-Consequently, these time services shall expire when the requested relative
-interval elapses, independently of the new or old value of the clock.
+Consequently,
+these time services shall expire when the requested duration elapses,
+independently of the new or old value of the clock.
.RE
.SH STANDARDS
POSIX.1-2008.
@@ -158,8 +159,8 @@ .SH HISTORY
This special extension was removed in Linux 2.5.39,
and is thus not available in Linux 2.6.0 and later kernels.
.SH NOTES
-If the interval specified in
-.I req
+If the
+.I duration
is not an exact multiple of the granularity underlying clock (see
.BR time (7)),
then the interval will be rounded up to the next multiple.
--
2.43.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* Re: [PATCH 2/2] clock_nanosleep.2, nanosleep.2: Use 'duration' rather than 'request'
From: Bruno Haible @ 2024-03-03 12:45 UTC (permalink / raw)
To: Alejandro Colomar
Cc: Elliott Hughes, Stefan Puiu, linux-man, GNU C Library, linux-api
In-Reply-To: <20240303121454.16994-3-alx@kernel.org>
Alejandro Colomar wrote:
> man2/clock_nanosleep.2 | 20 ++++++++++----------
> man2/nanosleep.2 | 12 ++++++------
The change to nanosleep.2 seems mostly fine. Except that the
term "requested relative duration" (line 142) raises questions;
what about changing that to "requested duration"?
The change to clock_nanosleep.2 seems wrong. There are two cases
(quoting the old text):
If flags is 0, then the value specified in request is interpreted
as an interval relative to the current value of the clock
specified by clockid.
If flags is TIMER_ABSTIME, then request is interpreted as an
absolute time as measured by the clock, clockid. If request is
less than or equal to the current value of the clock, then
clock_nanosleep() returns immediately without suspending the calling
thread.
In the first case, the argument is a duration. In the second case, the
argument is an absolute time point; it would be wrong and very confusing
to denote it as "duration".
Bruno
^ permalink raw reply
* Re: [PATCH 2/2] clock_nanosleep.2, nanosleep.2: Use 'duration' rather than 'request'
From: enh @ 2024-03-05 0:18 UTC (permalink / raw)
To: Alejandro Colomar
Cc: Bruno Haible, Stefan Puiu, linux-man, GNU C Library, linux-api
In-Reply-To: <ZeRzS6mENO8kOh1W@debian>
On Sun, Mar 3, 2024 at 4:55 AM Alejandro Colomar <alx@kernel.org> wrote:
>
> Hi Bruno,
>
> On Sun, Mar 03, 2024 at 01:45:37PM +0100, Bruno Haible wrote:
> > Alejandro Colomar wrote:
> > > man2/clock_nanosleep.2 | 20 ++++++++++----------
> > > man2/nanosleep.2 | 12 ++++++------
> >
> > The change to nanosleep.2 seems mostly fine. Except that the
> > term "requested relative duration" (line 142) raises questions;
> > what about changing that to "requested duration"?
>
> Yeah, I had doubts about that one. Probably I should drop 'relative'.
>
> >
> > The change to clock_nanosleep.2 seems wrong. There are two cases
> > (quoting the old text):
> >
> > If flags is 0, then the value specified in request is interpreted
> > as an interval relative to the current value of the clock
> > specified by clockid.
> >
> > If flags is TIMER_ABSTIME, then request is interpreted as an
> > absolute time as measured by the clock, clockid. If request is
> > less than or equal to the current value of the clock, then
> > clock_nanosleep() returns immediately without suspending the calling
> > thread.
> >
> > In the first case, the argument is a duration. In the second case, the
> > argument is an absolute time point; it would be wrong and very confusing
> > to denote it as "duration".
>
> Hmm, thanks! I guess we'll have to keep 'request' in clock_nanosleep(3)
> unless someone comes up with a better name. Elliott, you may want to
> partially revert that change in bionic.
thanks! https://android-review.googlesource.com/c/platform/bionic/+/2987070
changes to
/**
* [clock_nanosleep(2)](http://man7.org/linux/man-pages/man2/clock_nanosleep.2.html)
* sleeps for the given time (or until the given time if the TIMER_ABSTIME flag
* is used), as measured by the given clock.
*
* Returns 0 on success, and returns -1 and returns an error number on failure.
* If the sleep was interrupted by a signal, the return value will be `EINTR`
* and `remainder` will be the amount of time remaining.
*/
int clock_nanosleep(clockid_t __clock, int __flags, const struct
timespec* _Nonnull __time, struct timespec* _Nullable __remainder);
> Have a lovely day!
> Alex
>
> --
> <https://www.alejandro-colomar.es/>
> Looking for a remote C programming job at the moment.
^ permalink raw reply
* Re: [PATCH 2/2] clock_nanosleep.2, nanosleep.2: Use 'duration' rather than 'request'
From: Alejandro Colomar @ 2024-03-05 0:34 UTC (permalink / raw)
To: enh; +Cc: Bruno Haible, Stefan Puiu, linux-man, GNU C Library, linux-api
In-Reply-To: <CAJgzZor8TTSysM=TiTXQdVtHMZPQWu5YOhPmb8PAevdVd-c31Q@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1131 bytes --]
Hi Elliott,
On Mon, Mar 04, 2024 at 04:18:28PM -0800, enh wrote:
> thanks! https://android-review.googlesource.com/c/platform/bionic/+/2987070
> changes to
>
> /**
> * [clock_nanosleep(2)](http://man7.org/linux/man-pages/man2/clock_nanosleep.2.html)
> * sleeps for the given time (or until the given time if the TIMER_ABSTIME flag
> * is used), as measured by the given clock.
> *
> * Returns 0 on success, and returns -1 and returns an error number on failure.
> * If the sleep was interrupted by a signal, the return value will be `EINTR`
> * and `remainder` will be the amount of time remaining.
> */
> int clock_nanosleep(clockid_t __clock, int __flags, const struct
> timespec* _Nonnull __time, struct timespec* _Nullable __remainder);
Hmmmm, that's the best name, meaningfully, I think. But I've been
trying to avoid it. I don't like using names of standard functions in
identifiers; it might confuse. As an alternative, I thought of 't'.
What do you think?
Have a lovely night!
Alex
--
<https://www.alejandro-colomar.es/>
Looking for a remote C programming job at the moment.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 2/2] clock_nanosleep.2, nanosleep.2: Use 'duration' rather than 'request'
From: enh @ 2024-03-05 0:56 UTC (permalink / raw)
To: Alejandro Colomar
Cc: Bruno Haible, Stefan Puiu, linux-man, GNU C Library, linux-api
In-Reply-To: <ZeZohz1sLcIN6kxA@debian>
On Mon, Mar 4, 2024 at 4:34 PM Alejandro Colomar <alx@kernel.org> wrote:
>
> Hi Elliott,
>
> On Mon, Mar 04, 2024 at 04:18:28PM -0800, enh wrote:
> > thanks! https://android-review.googlesource.com/c/platform/bionic/+/2987070
> > changes to
> >
> > /**
> > * [clock_nanosleep(2)](http://man7.org/linux/man-pages/man2/clock_nanosleep.2.html)
> > * sleeps for the given time (or until the given time if the TIMER_ABSTIME flag
> > * is used), as measured by the given clock.
> > *
> > * Returns 0 on success, and returns -1 and returns an error number on failure.
> > * If the sleep was interrupted by a signal, the return value will be `EINTR`
> > * and `remainder` will be the amount of time remaining.
> > */
> > int clock_nanosleep(clockid_t __clock, int __flags, const struct
> > timespec* _Nonnull __time, struct timespec* _Nullable __remainder);
>
> Hmmmm, that's the best name, meaningfully, I think. But I've been
> trying to avoid it. I don't like using names of standard functions in
> identifiers; it might confuse. As an alternative, I thought of 't'.
> What do you think?
as you can see, i've taken the "the leading `__` means we get to
trample whatever we like" approach :-)
(we build bionic with hidden visibility and an explicit list of
symbols for the linker to export, so we'd have to be trying quite hard
to trip over ourselves.)
> Have a lovely night!
>
> Alex
>
> --
> <https://www.alejandro-colomar.es/>
> Looking for a remote C programming job at the moment.
^ permalink raw reply
* Re: [PATCH 2/2] clock_nanosleep.2, nanosleep.2: Use 'duration' rather than 'request'
From: Alejandro Colomar @ 2024-03-05 1:11 UTC (permalink / raw)
To: enh; +Cc: Bruno Haible, Stefan Puiu, linux-man, GNU C Library, linux-api
In-Reply-To: <CAJgzZoog1qS4BOYaKDnLsA3RzL-61r=33tP-XK2xvOwa008jJw@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 889 bytes --]
On Mon, Mar 04, 2024 at 04:56:13PM -0800, enh wrote:
> > > int clock_nanosleep(clockid_t __clock, int __flags, const struct
> > > timespec* _Nonnull __time, struct timespec* _Nullable __remainder);
> >
> > Hmmmm, that's the best name, meaningfully, I think. But I've been
> > trying to avoid it. I don't like using names of standard functions in
> > identifiers; it might confuse. As an alternative, I thought of 't'.
> > What do you think?
>
> as you can see, i've taken the "the leading `__` means we get to
> trample whatever we like" approach :-)
>
> (we build bionic with hidden visibility and an explicit list of
> symbols for the linker to export, so we'd have to be trying quite hard
> to trip over ourselves.)
Yeah, I was worried about the manual page :)
--
<https://www.alejandro-colomar.es/>
Looking for a remote C programming job at the moment.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 1/2] man*/: epoll_*(), fcntl(), flock(), ioctl(), msgctl(), *prctl(), ptrace(), quotactl(), reboot(), semctl(), shmctl(), lockf(): Consistently use 'op' and 'operation'
From: Alejandro Colomar @ 2024-03-05 18:12 UTC (permalink / raw)
To: Elliott Hughes, Stefan Puiu, Bruno Haible, linux-man
Cc: GNU C Library, linux-api
In-Reply-To: <20240303121454.16994-2-alx@kernel.org>
[-- Attachment #1: Type: text/plain, Size: 2043 bytes --]
On Sun, Mar 03, 2024 at 01:15:09PM +0100, Alejandro Colomar wrote:
> Reported-by: Bruno Haible <bruno@clisp.org>
> Cc: Elliott Hughes <enh@google.com>
> Cc: Stefan Puiu <stefan.puiu@gmail.com>
> Signed-off-by: Alejandro Colomar <alx@kernel.org>
> ---
> man2/arch_prctl.2 | 12 +++---
> man2/epoll_wait.2 | 4 +-
> man2/fcntl.2 | 70 +++++++++++++++----------------
> man2/flock.2 | 6 +--
> man2/ioctl.2 | 30 +++++++-------
> man2/ioctl_console.2 | 8 ++--
> man2/ioctl_fideduperange.2 | 2 +-
> man2/ioctl_getfsmap.2 | 6 +--
> man2/ioctl_ns.2 | 2 +-
> man2/ioctl_tty.2 | 10 ++---
> man2/ioctl_userfaultfd.2 | 10 ++---
> man2/msgctl.2 | 16 ++++----
> man2/prctl.2 | 84 +++++++++++++++++++-------------------
> man2/ptrace.2 | 60 +++++++++++++--------------
> man2/quotactl.2 | 34 +++++++--------
> man2/reboot.2 | 19 +++++----
> man2/semctl.2 | 22 +++++-----
> man2/shmctl.2 | 20 +++++----
> man3/lockf.3 | 11 ++---
> 19 files changed, 217 insertions(+), 209 deletions(-)
>
> diff --git a/man2/arch_prctl.2 b/man2/arch_prctl.2
> index bbb85866c..680d36395 100644
> --- a/man2/arch_prctl.2
> +++ b/man2/arch_prctl.2
> @@ -14,8 +14,8 @@ .SH SYNOPSIS
> .BR "#include <sys/syscall.h>" " /* Definition of " SYS_* " constants */"
> .B #include <unistd.h>
> .P
> -.BI "int syscall(SYS_arch_prctl, int " code ", unsigned long " addr );
> -.BI "int syscall(SYS_arch_prctl, int " code ", unsigned long *" addr );
> +.BI "int syscall(SYS_arch_prctl, int " op ", unsigned long " addr );
> +.BI "int syscall(SYS_arch_prctl, int " op ", unsigned long *" addr );
[...]
Hi Elliott!
This is already in master. Feel free to do a similar thing in bionic.
;)
Have a lovely day!
Alex
--
<https://www.alejandro-colomar.es/>
Looking for a remote C programming job at the moment.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 1/2] man*/: epoll_*(), fcntl(), flock(), ioctl(), msgctl(), *prctl(), ptrace(), quotactl(), reboot(), semctl(), shmctl(), lockf(): Consistently use 'op' and 'operation'
From: enh @ 2024-03-05 19:19 UTC (permalink / raw)
To: Alejandro Colomar
Cc: Stefan Puiu, Bruno Haible, linux-man, GNU C Library, linux-api
In-Reply-To: <ZedgnagXMV5aTcsM@debian>
On Tue, Mar 5, 2024 at 10:12 AM Alejandro Colomar <alx@kernel.org> wrote:
>
> On Sun, Mar 03, 2024 at 01:15:09PM +0100, Alejandro Colomar wrote:
> > Reported-by: Bruno Haible <bruno@clisp.org>
> > Cc: Elliott Hughes <enh@google.com>
> > Cc: Stefan Puiu <stefan.puiu@gmail.com>
> > Signed-off-by: Alejandro Colomar <alx@kernel.org>
> > ---
> > man2/arch_prctl.2 | 12 +++---
> > man2/epoll_wait.2 | 4 +-
> > man2/fcntl.2 | 70 +++++++++++++++----------------
> > man2/flock.2 | 6 +--
> > man2/ioctl.2 | 30 +++++++-------
> > man2/ioctl_console.2 | 8 ++--
> > man2/ioctl_fideduperange.2 | 2 +-
> > man2/ioctl_getfsmap.2 | 6 +--
> > man2/ioctl_ns.2 | 2 +-
> > man2/ioctl_tty.2 | 10 ++---
> > man2/ioctl_userfaultfd.2 | 10 ++---
> > man2/msgctl.2 | 16 ++++----
> > man2/prctl.2 | 84 +++++++++++++++++++-------------------
> > man2/ptrace.2 | 60 +++++++++++++--------------
> > man2/quotactl.2 | 34 +++++++--------
> > man2/reboot.2 | 19 +++++----
> > man2/semctl.2 | 22 +++++-----
> > man2/shmctl.2 | 20 +++++----
> > man3/lockf.3 | 11 ++---
> > 19 files changed, 217 insertions(+), 209 deletions(-)
> >
> > diff --git a/man2/arch_prctl.2 b/man2/arch_prctl.2
> > index bbb85866c..680d36395 100644
> > --- a/man2/arch_prctl.2
> > +++ b/man2/arch_prctl.2
> > @@ -14,8 +14,8 @@ .SH SYNOPSIS
> > .BR "#include <sys/syscall.h>" " /* Definition of " SYS_* " constants */"
> > .B #include <unistd.h>
> > .P
> > -.BI "int syscall(SYS_arch_prctl, int " code ", unsigned long " addr );
> > -.BI "int syscall(SYS_arch_prctl, int " code ", unsigned long *" addr );
> > +.BI "int syscall(SYS_arch_prctl, int " op ", unsigned long " addr );
> > +.BI "int syscall(SYS_arch_prctl, int " op ", unsigned long *" addr );
>
> [...]
>
> Hi Elliott!
>
> This is already in master. Feel free to do a similar thing in bionic.
> ;)
already did :-)
https://android-review.googlesource.com/c/platform/bionic/+/2987969
> Have a lovely day!
> Alex
>
> --
> <https://www.alejandro-colomar.es/>
> Looking for a remote C programming job at the moment.
^ permalink raw reply
* Re: [PATCH 2/2] clock_nanosleep.2, nanosleep.2: Use 'duration' rather than 'request'
From: enh @ 2024-03-05 22:22 UTC (permalink / raw)
To: Alejandro Colomar
Cc: Bruno Haible, Stefan Puiu, linux-man, GNU C Library, linux-api
In-Reply-To: <ZeZxSydsfskaQ5Vw@debian>
On Mon, Mar 4, 2024 at 5:11 PM Alejandro Colomar <alx@kernel.org> wrote:
>
> On Mon, Mar 04, 2024 at 04:56:13PM -0800, enh wrote:
> > > > int clock_nanosleep(clockid_t __clock, int __flags, const struct
> > > > timespec* _Nonnull __time, struct timespec* _Nullable __remainder);
> > >
> > > Hmmmm, that's the best name, meaningfully, I think. But I've been
> > > trying to avoid it. I don't like using names of standard functions in
> > > identifiers; it might confuse. As an alternative, I thought of 't'.
> > > What do you think?
> >
> > as you can see, i've taken the "the leading `__` means we get to
> > trample whatever we like" approach :-)
> >
> > (we build bionic with hidden visibility and an explicit list of
> > symbols for the linker to export, so we'd have to be trying quite hard
> > to trip over ourselves.)
>
> Yeah, I was worried about the manual page :)
yeah, i think "t + extra text" makes sense there. i just try to be as
brief as possible in the doc comments on the assumption that most
readers will be seeing them in IDE pop-ups, and anyone who wants lots
of text will click through to the man page anyway. and at that point
they're your problem :-)
> --
> <https://www.alejandro-colomar.es/>
> Looking for a remote C programming job at the moment.
^ permalink raw reply
* Re: [PATCH v2 02/31] ntsync: Introduce NTSYNC_IOC_CREATE_SEM.
From: Greg Kroah-Hartman @ 2024-03-07 22:12 UTC (permalink / raw)
To: Elizabeth Figura
Cc: Arnd Bergmann, Jonathan Corbet, Shuah Khan, linux-kernel,
linux-api, wine-devel, André Almeida, Wolfram Sang,
Arkadiusz Hiler, Peter Zijlstra, Andy Lutomirski, linux-doc,
linux-kselftest, Randy Dunlap
In-Reply-To: <20240219223833.95710-3-zfigura@codeweavers.com>
On Mon, Feb 19, 2024 at 04:38:04PM -0600, Elizabeth Figura wrote:
> This corresponds to the NT syscall NtCreateSemaphore().
>
> Semaphores are one of three types of object to be implemented in this driver,
> the others being mutexes and events.
>
> An NT semaphore contains a 32-bit counter, and is signaled and can be acquired
> when the counter is nonzero. The counter has a maximum value which is specified
> at creation time. The initial value of the semaphore is also specified at
> creation time. There are no restrictions on the maximum and initial value.
>
> Each object is exposed as an file, to which any number of fds may be opened.
> When all fds are closed, the object is deleted.
>
> Objects hold a pointer to the ntsync_device that created them. The device's
> reference count is driven by struct file.
>
> Signed-off-by: Elizabeth Figura <zfigura@codeweavers.com>
I want to take these, but I need someone who knows how to review
locking/sync primitives to give it a review to verify that it is working
properly within the kernel constraints we have here.
Anyone want to give it a review?
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v10 2/9] landlock: Add IOCTL access right for character and block devices
From: Mickaël Salaün @ 2024-03-11 14:46 UTC (permalink / raw)
To: Günther Noack, Alejandro Colomar, Jonathan Corbet
Cc: linux-security-module, Jeff Xu, Arnd Bergmann,
Jorge Lucangeli Obes, Allen Webb, Dmitry Torokhov, Paul Moore,
Konstantin Meskhidze, Matt Bobrowski, linux-fsdevel,
Christian Brauner, linux-api
In-Reply-To: <20240309075320.160128-3-gnoack@google.com>
Adding Alex, Jon and the API ML for interface-related question: file
type check or not?
On Sat, Mar 09, 2024 at 07:53:13AM +0000, Günther Noack wrote:
> Introduces the LANDLOCK_ACCESS_FS_IOCTL_DEV right
> and increments the Landlock ABI version to 5.
>
> This access right applies to device-custom IOCTL commands
> when they are invoked on block or character device files.
>
> Like the truncate right, this right is associated with a file
> descriptor at the time of open(2), and gets respected even when the
> file descriptor is used outside of the thread which it was originally
> opened in.
>
> Therefore, a newly enabled Landlock policy does not apply to file
> descriptors which are already open.
>
> If the LANDLOCK_ACCESS_FS_IOCTL_DEV right is handled, only a small
> number of safe IOCTL commands will be permitted on newly opened device
> files. These include FIOCLEX, FIONCLEX, FIONBIO and FIOASYNC, as well
> as other IOCTL commands for regular files which are implemented in
> fs/ioctl.c.
>
> Noteworthy scenarios which require special attention:
>
> TTY devices are often passed into a process from the parent process,
> and so a newly enabled Landlock policy does not retroactively apply to
> them automatically. In the past, TTY devices have often supported
> IOCTL commands like TIOCSTI and some TIOCLINUX subcommands, which were
> letting callers control the TTY input buffer (and simulate
> keypresses). This should be restricted to CAP_SYS_ADMIN programs on
> modern kernels though.
>
> Known limitations:
>
> The LANDLOCK_ACCESS_FS_IOCTL_DEV access right is a coarse-grained
> control over IOCTL commands.
>
> Landlock users may use path-based restrictions in combination with
> their knowledge about the file system layout to control what IOCTLs
> can be done.
A few minor (or not) nitpicks, but overall I really like this series.
>
> Cc: Paul Moore <paul@paul-moore.com>
> Cc: Christian Brauner <brauner@kernel.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Günther Noack <gnoack@google.com>
> ---
> include/uapi/linux/landlock.h | 35 +++++++++++++-----
> security/landlock/fs.c | 38 ++++++++++++++++++--
> security/landlock/limits.h | 2 +-
> security/landlock/syscalls.c | 8 +++--
> tools/testing/selftests/landlock/base_test.c | 2 +-
> tools/testing/selftests/landlock/fs_test.c | 5 +--
> 6 files changed, 73 insertions(+), 17 deletions(-)
>
> diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
> index 25c8d7677539..193733d833b1 100644
> --- a/include/uapi/linux/landlock.h
> +++ b/include/uapi/linux/landlock.h
> @@ -128,7 +128,7 @@ struct landlock_net_port_attr {
> * files and directories. Files or directories opened before the sandboxing
> * are not subject to these restrictions.
> *
> - * A file can only receive these access rights:
> + * The following access rights apply only to files:
> *
> * - %LANDLOCK_ACCESS_FS_EXECUTE: Execute a file.
> * - %LANDLOCK_ACCESS_FS_WRITE_FILE: Open a file with write access. Note that
> @@ -138,12 +138,13 @@ struct landlock_net_port_attr {
> * - %LANDLOCK_ACCESS_FS_READ_FILE: Open a file with read access.
> * - %LANDLOCK_ACCESS_FS_TRUNCATE: Truncate a file with :manpage:`truncate(2)`,
> * :manpage:`ftruncate(2)`, :manpage:`creat(2)`, or :manpage:`open(2)` with
> - * ``O_TRUNC``. Whether an opened file can be truncated with
> - * :manpage:`ftruncate(2)` is determined during :manpage:`open(2)`, in the
> - * same way as read and write permissions are checked during
> - * :manpage:`open(2)` using %LANDLOCK_ACCESS_FS_READ_FILE and
> - * %LANDLOCK_ACCESS_FS_WRITE_FILE. This access right is available since the
> - * third version of the Landlock ABI.
> + * ``O_TRUNC``. This access right is available since the third version of the
> + * Landlock ABI.
> + *
> + * Whether an opened file can be truncated with :manpage:`ftruncate(2)` or used
> + * with `ioctl(2)` is determined during :manpage:`open(2)`, in the same way as
> + * read and write permissions are checked during :manpage:`open(2)` using
> + * %LANDLOCK_ACCESS_FS_READ_FILE and %LANDLOCK_ACCESS_FS_WRITE_FILE.
> *
> * A directory can receive access rights related to files or directories. The
> * following access right is applied to the directory itself, and the
> @@ -198,13 +199,30 @@ struct landlock_net_port_attr {
> * If multiple requirements are not met, the ``EACCES`` error code takes
> * precedence over ``EXDEV``.
> *
> + * The following access right applies both to files and directories:
> + *
> + * - %LANDLOCK_ACCESS_FS_IOCTL_DEV: Invoke :manpage:`ioctl(2)` commands on an opened
> + * character or block device.
> + *
> + * This access right applies to all `ioctl(2)` commands implemented by device
> + * drivers. However, the following common IOCTL commands continue to be
> + * invokable independent of the %LANDLOCK_ACCESS_FS_IOCTL_DEV right:
> + *
> + * ``FIOCLEX``, ``FIONCLEX``, ``FIONBIO``, ``FIOASYNC``, ``FIOQSIZE``,
> + * ``FIFREEZE``, ``FITHAW``, ``FS_IOC_FIEMAP``, ``FIGETBSZ``, ``FICLONE``,
> + * ``FICLONERANGE``, ``FIDEDUPERANGE``, ``FS_IOC_GETFLAGS``,
> + * ``FS_IOC_SETFLAGS``, ``FS_IOC_FSGETXATTR``, ``FS_IOC_FSSETXATTR``
> + *
> + * This access right is available since the fifth version of the Landlock
> + * ABI.
> + *
> * .. warning::
> *
> * It is currently not possible to restrict some file-related actions
> * accessible through these syscall families: :manpage:`chdir(2)`,
> * :manpage:`stat(2)`, :manpage:`flock(2)`, :manpage:`chmod(2)`,
> * :manpage:`chown(2)`, :manpage:`setxattr(2)`, :manpage:`utime(2)`,
> - * :manpage:`ioctl(2)`, :manpage:`fcntl(2)`, :manpage:`access(2)`.
> + * :manpage:`fcntl(2)`, :manpage:`access(2)`.
> * Future Landlock evolutions will enable to restrict them.
> */
> /* clang-format off */
> @@ -223,6 +241,7 @@ struct landlock_net_port_attr {
> #define LANDLOCK_ACCESS_FS_MAKE_SYM (1ULL << 12)
> #define LANDLOCK_ACCESS_FS_REFER (1ULL << 13)
> #define LANDLOCK_ACCESS_FS_TRUNCATE (1ULL << 14)
> +#define LANDLOCK_ACCESS_FS_IOCTL_DEV (1ULL << 15)
> /* clang-format on */
>
> /**
> diff --git a/security/landlock/fs.c b/security/landlock/fs.c
> index 6f0bf1434a2c..bfa69ea94cf8 100644
> --- a/security/landlock/fs.c
> +++ b/security/landlock/fs.c
> @@ -148,7 +148,8 @@ static struct landlock_object *get_inode_object(struct inode *const inode)
> LANDLOCK_ACCESS_FS_EXECUTE | \
> LANDLOCK_ACCESS_FS_WRITE_FILE | \
> LANDLOCK_ACCESS_FS_READ_FILE | \
> - LANDLOCK_ACCESS_FS_TRUNCATE)
> + LANDLOCK_ACCESS_FS_TRUNCATE | \
> + LANDLOCK_ACCESS_FS_IOCTL_DEV)
We may want to check the file type to make sure we set the
LANDLOCK_ACCESS_FS_IOCTL_DEV right on char/block devices only, the same
way we already check with d_is_dir() [1]. From user space point of
view, it should not change much because a call to statfs(2) may already
be in place. From kernel space point of view it would only be a matter
of checking the related inode in landlock_append_fs_rule().
Checking for the file type is not strictly necessarily, but I
implemented the d_is_dir() call and get_path_from_fd() checks to
encourage/force user space to check the file/directory on which it wants
to give access to (e.g. and not erroneously grant access to a whole file
hierarchy rather than a file thanks to statfs(2) information, not the
Landlock syscall itself). Applications sandboxing themselves should not
be surprise that a file descriptor refers to a directory or a file, and
they should not require additional call to statfs(2). Another
motivation was that I think this kind of conservative check would have
been difficult to implement later (with an option) because of the
potential user space architectural changes. Finally, this kind of type
checking can be silently ignored with help from user space libraries
when needed.
About the char/block device check, it might also be a good idea for user
space to check the major/minor numbers to make sure they match
expectations (i.e. related IOCTL commands).
I'm convinced the get_path_from_fd() checks are good because special
files are not restricted (and can then be silently ignored without
impact), whereas a non-special file could still get a valid (super)set
of access rights (and maybe better follow the principle of least
astonishment?). I'm wondering if checking dir/file was the best
decision, if this is enough, or if we should extend that to char/block
devices. Any opinion an that?
[1] https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/security/landlock/fs.c?h=v6.8#n166
> /* clang-format on */
>
> /*
> @@ -1332,8 +1333,10 @@ static int hook_file_alloc_security(struct file *const file)
> static int hook_file_open(struct file *const file)
> {
> layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS] = {};
> - access_mask_t open_access_request, full_access_request, allowed_access;
> - const access_mask_t optional_access = LANDLOCK_ACCESS_FS_TRUNCATE;
> + access_mask_t open_access_request, full_access_request, allowed_access,
> + optional_access;
> + const struct inode *inode = file_inode(file);
> + const bool is_device = S_ISBLK(inode->i_mode) || S_ISCHR(inode->i_mode);
> const struct landlock_ruleset *const dom = get_current_fs_domain();
>
> if (!dom)
> @@ -1350,6 +1353,10 @@ static int hook_file_open(struct file *const file)
> * We look up more access than what we immediately need for open(), so
> * that we can later authorize operations on opened files.
> */
> + optional_access = LANDLOCK_ACCESS_FS_TRUNCATE;
> + if (is_device)
> + optional_access |= LANDLOCK_ACCESS_FS_IOCTL_DEV;
> +
> full_access_request = open_access_request | optional_access;
>
> if (is_access_to_paths_allowed(
> @@ -1406,6 +1413,30 @@ static int hook_file_truncate(struct file *const file)
> return -EACCES;
> }
>
> +static int hook_file_vfs_ioctl(struct file *file, unsigned int cmd,
> + unsigned long arg)
> +{
> + const struct inode *inode = file_inode(file);
> + const bool is_device = S_ISBLK(inode->i_mode) || S_ISCHR(inode->i_mode);
> + access_mask_t required_access, allowed_access;
> +
> + if (!is_device)
> + return 0;
We should first check landlock_file(file)->allowed_access as in
hook_file_truncate() to return as soon as possible for non-sandboxed
tasks. Any other computation should be done after that (e.g. with an
is_device() helper).
> +
> + /*
> + * It is the access rights at the time of opening the file which
> + * determine whether IOCTL can be used on the opened file later.
> + *
> + * The access right is attached to the opened file in hook_file_open().
> + */
> + required_access = LANDLOCK_ACCESS_FS_IOCTL_DEV;
> + allowed_access = landlock_file(file)->allowed_access;
> + if ((allowed_access & required_access) == required_access)
> + return 0;
> +
> + return -EACCES;
> +}
> +
> static struct security_hook_list landlock_hooks[] __ro_after_init = {
> LSM_HOOK_INIT(inode_free_security, hook_inode_free_security),
>
> @@ -1428,6 +1459,7 @@ static struct security_hook_list landlock_hooks[] __ro_after_init = {
> LSM_HOOK_INIT(file_alloc_security, hook_file_alloc_security),
> LSM_HOOK_INIT(file_open, hook_file_open),
> LSM_HOOK_INIT(file_truncate, hook_file_truncate),
> + LSM_HOOK_INIT(file_vfs_ioctl, hook_file_vfs_ioctl),
> };
>
> __init void landlock_add_fs_hooks(void)
> diff --git a/security/landlock/limits.h b/security/landlock/limits.h
> index 93c9c6f91556..20fdb5ff3514 100644
> --- a/security/landlock/limits.h
> +++ b/security/landlock/limits.h
> @@ -18,7 +18,7 @@
> #define LANDLOCK_MAX_NUM_LAYERS 16
> #define LANDLOCK_MAX_NUM_RULES U32_MAX
>
> -#define LANDLOCK_LAST_ACCESS_FS LANDLOCK_ACCESS_FS_TRUNCATE
> +#define LANDLOCK_LAST_ACCESS_FS LANDLOCK_ACCESS_FS_IOCTL_DEV
> #define LANDLOCK_MASK_ACCESS_FS ((LANDLOCK_LAST_ACCESS_FS << 1) - 1)
> #define LANDLOCK_NUM_ACCESS_FS __const_hweight64(LANDLOCK_MASK_ACCESS_FS)
> #define LANDLOCK_SHIFT_ACCESS_FS 0
> diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c
> index 6788e73b6681..9ae3dfa47443 100644
> --- a/security/landlock/syscalls.c
> +++ b/security/landlock/syscalls.c
> @@ -149,7 +149,7 @@ static const struct file_operations ruleset_fops = {
> .write = fop_dummy_write,
> };
>
> -#define LANDLOCK_ABI_VERSION 4
> +#define LANDLOCK_ABI_VERSION 5
>
> /**
> * sys_landlock_create_ruleset - Create a new ruleset
> @@ -321,7 +321,11 @@ static int add_rule_path_beneath(struct landlock_ruleset *const ruleset,
> if (!path_beneath_attr.allowed_access)
> return -ENOMSG;
>
> - /* Checks that allowed_access matches the @ruleset constraints. */
> + /*
> + * Checks that allowed_access matches the @ruleset constraints and only
> + * consists of publicly visible access rights (as opposed to synthetic
> + * ones).
> + */
This change is not needed anymore.
> mask = landlock_get_raw_fs_access_mask(ruleset, 0);
> if ((path_beneath_attr.allowed_access | mask) != mask)
> return -EINVAL;
> diff --git a/tools/testing/selftests/landlock/base_test.c b/tools/testing/selftests/landlock/base_test.c
> index 646f778dfb1e..d292b419ccba 100644
> --- a/tools/testing/selftests/landlock/base_test.c
> +++ b/tools/testing/selftests/landlock/base_test.c
> @@ -75,7 +75,7 @@ TEST(abi_version)
> const struct landlock_ruleset_attr ruleset_attr = {
> .handled_access_fs = LANDLOCK_ACCESS_FS_READ_FILE,
> };
> - ASSERT_EQ(4, landlock_create_ruleset(NULL, 0,
> + ASSERT_EQ(5, landlock_create_ruleset(NULL, 0,
> LANDLOCK_CREATE_RULESET_VERSION));
>
> ASSERT_EQ(-1, landlock_create_ruleset(&ruleset_attr, 0,
> diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
> index 2d6d9b43d958..0bcbbf594fd7 100644
> --- a/tools/testing/selftests/landlock/fs_test.c
> +++ b/tools/testing/selftests/landlock/fs_test.c
> @@ -527,9 +527,10 @@ TEST_F_FORK(layout1, inval)
> LANDLOCK_ACCESS_FS_EXECUTE | \
> LANDLOCK_ACCESS_FS_WRITE_FILE | \
> LANDLOCK_ACCESS_FS_READ_FILE | \
> - LANDLOCK_ACCESS_FS_TRUNCATE)
> + LANDLOCK_ACCESS_FS_TRUNCATE | \
> + LANDLOCK_ACCESS_FS_IOCTL_DEV)
>
> -#define ACCESS_LAST LANDLOCK_ACCESS_FS_TRUNCATE
> +#define ACCESS_LAST LANDLOCK_ACCESS_FS_IOCTL_DEV
>
> #define ACCESS_ALL ( \
> ACCESS_FILE | \
> --
> 2.44.0.278.ge034bb2e1d-goog
>
^ permalink raw reply
* Re: [PATCH v10 2/9] landlock: Add IOCTL access right for character and block devices
From: Alejandro Colomar @ 2024-03-11 16:55 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Günther Noack, Jonathan Corbet, linux-security-module,
Jeff Xu, Arnd Bergmann, Jorge Lucangeli Obes, Allen Webb,
Dmitry Torokhov, Paul Moore, Konstantin Meskhidze, Matt Bobrowski,
linux-fsdevel, Christian Brauner, linux-api
In-Reply-To: <20240311.If7ieshaegu2@digikod.net>
[-- Attachment #1: Type: text/plain, Size: 336 bytes --]
On Mon, Mar 11, 2024 at 03:46:36PM +0100, Mickaël Salaün wrote:
> Adding Alex, Jon and the API ML for interface-related question: file
> type check or not?
Hi Mickaël!
I'm sorry, I don't know. :)
Have a lovely day!
Alex
--
<https://www.alejandro-colomar.es/>
Looking for a remote C programming job at the moment.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v2 04/31] ntsync: Introduce NTSYNC_IOC_WAIT_ANY.
From: Elizabeth Figura @ 2024-03-11 18:58 UTC (permalink / raw)
To: Arnd Bergmann, Greg Kroah-Hartman, Jonathan Corbet, Shuah Khan
Cc: linux-kernel, linux-api, wine-devel, André Almeida,
Wolfram Sang, Arkadiusz Hiler, Peter Zijlstra, Andy Lutomirski,
linux-doc, linux-kselftest, Randy Dunlap
In-Reply-To: <20240219223833.95710-5-zfigura@codeweavers.com>
On Monday, 19 February 2024 16:38:06 CDT Elizabeth Figura wrote:
> +static struct ntsync_obj *get_obj(struct ntsync_device *dev, int fd)
> +{
> + struct file *file = fget(fd);
> + struct ntsync_obj *obj;
> +
> + if (file->f_op != &ntsync_obj_fops) {
> + fput(file);
> + return NULL;
> + }
I just noticed during self-review that this fails to check the result of
fget() for NULL :-/
I'll fix this next revision.
> +
> + obj = file->private_data;
> + if (obj->dev != dev) {
> + fput(file);
> + return NULL;
> + }
> +
> + return obj;
> +}
^ permalink raw reply
* [PATCH v6] posix-timers: add clock_compare system call
From: Sagi Maimon @ 2024-03-12 9:50 UTC (permalink / raw)
To: richardcochran, luto, datglx, mingo, bp, dave.hansen, x86, hpa,
arnd, geert, peterz, hannes, sohil.mehta, rick.p.edgecombe,
nphamcs, palmer, maimon.sagi, keescook, legion, mark.rutland,
mszeredi, casey, reibax, davem, brauner
Cc: linux-kernel, linux-api, linux-arch, netdev
Some user space applications need to read a couple of different clocks.
Each read requires moving from user space to kernel space.
Reading each clock separately (syscall) introduces extra
unpredictable/unmeasurable delay. Minimizing this delay contributes to user
space actions on these clocks (e.g. synchronization etc).
Introduce a new system call clock_compare, which can be used to measure
the offset between two clocks, from variety of types: PHC, virtual PHC
and various system clocks (CLOCK_REALTIME, CLOCK_MONOTONIC, etc).
The system call returns the clocks timestamps.
When possible, use crosstimespec to sync read values.
Else, read clock A twice (before, and after reading clock B) and average these
times – to be as close as possible to the time we read clock B.
Signed-off-by: Sagi Maimon <maimon.sagi@gmail.com>
---
Addressed comments from:
- Richard Cochran : https://www.spinics.net/lists/netdev/msg964410.html
Changes since version 5:
- take only two clocks time samples
- use crosstimespec if supported
arch/x86/entry/syscalls/syscall_64.tbl | 1 +
drivers/ptp/ptp_clock.c | 34 ++++--
include/linux/posix-clock.h | 2 +
include/linux/syscalls.h | 4 +
include/uapi/asm-generic/unistd.h | 5 +-
kernel/time/posix-clock.c | 25 +++++
kernel/time/posix-timers.c | 145 +++++++++++++++++++++++++
kernel/time/posix-timers.h | 2 +
8 files changed, 207 insertions(+), 11 deletions(-)
diff --git a/arch/x86/entry/syscalls/syscall_64.tbl b/arch/x86/entry/syscalls/syscall_64.tbl
index 7e8d46f4147f..727930d27e05 100644
--- a/arch/x86/entry/syscalls/syscall_64.tbl
+++ b/arch/x86/entry/syscalls/syscall_64.tbl
@@ -383,6 +383,7 @@
459 common lsm_get_self_attr sys_lsm_get_self_attr
460 common lsm_set_self_attr sys_lsm_set_self_attr
461 common lsm_list_modules sys_lsm_list_modules
+462 common clock_compare sys_clock_compare
#
# Due to a historical design error, certain syscalls are numbered differently
diff --git a/drivers/ptp/ptp_clock.c b/drivers/ptp/ptp_clock.c
index 15b804ba4868..37ce66d4159f 100644
--- a/drivers/ptp/ptp_clock.c
+++ b/drivers/ptp/ptp_clock.c
@@ -156,17 +156,31 @@ static int ptp_clock_adjtime(struct posix_clock *pc, struct __kernel_timex *tx)
return err;
}
+static int ptp_clock_getcrosstime(struct posix_clock *pc, struct system_device_crosststamp *xtstamp)
+{
+ struct ptp_clock *ptp = container_of(pc, struct ptp_clock, clock);
+ int err;
+
+ if (!ptp->info->getcrosststamp)
+ err = -EOPNOTSUPP;
+ else
+ err = ptp->info->getcrosststamp(ptp->info, xtstamp);
+
+ return err;
+}
+
static struct posix_clock_operations ptp_clock_ops = {
- .owner = THIS_MODULE,
- .clock_adjtime = ptp_clock_adjtime,
- .clock_gettime = ptp_clock_gettime,
- .clock_getres = ptp_clock_getres,
- .clock_settime = ptp_clock_settime,
- .ioctl = ptp_ioctl,
- .open = ptp_open,
- .release = ptp_release,
- .poll = ptp_poll,
- .read = ptp_read,
+ .owner = THIS_MODULE,
+ .clock_adjtime = ptp_clock_adjtime,
+ .clock_gettime = ptp_clock_gettime,
+ .clock_getres = ptp_clock_getres,
+ .clock_settime = ptp_clock_settime,
+ .clock_getcrosstime = ptp_clock_getcrosstime,
+ .ioctl = ptp_ioctl,
+ .open = ptp_open,
+ .release = ptp_release,
+ .poll = ptp_poll,
+ .read = ptp_read,
};
static void ptp_clock_release(struct device *dev)
diff --git a/include/linux/posix-clock.h b/include/linux/posix-clock.h
index ef8619f48920..3a5b4bb3f56b 100644
--- a/include/linux/posix-clock.h
+++ b/include/linux/posix-clock.h
@@ -47,6 +47,8 @@ struct posix_clock_operations {
int (*clock_settime)(struct posix_clock *pc,
const struct timespec64 *ts);
+ int (*clock_getcrosstime)(struct posix_clock *pc,
+ struct system_device_crosststamp *xtstamp);
/*
* Optional character device methods:
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 77eb9b0e7685..ba2ce5b927aa 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -1188,6 +1188,10 @@ asmlinkage long sys_ni_syscall(void);
asmlinkage long sys_ni_posix_timers(void);
+asmlinkage long clock_compare(const clockid_t clock_a, const clockid_t clock_b,
+ struct __kernel_timespec __user *tp_a,
+ struct __kernel_timespec __user *tp_b,
+ int64_t __user *offs_err);
/*
* Kernel code should not call syscalls (i.e., sys_xyzyyz()) directly.
* Instead, use one of the functions which work equivalently, such as
diff --git a/include/uapi/asm-generic/unistd.h b/include/uapi/asm-generic/unistd.h
index 75f00965ab15..537a35afd237 100644
--- a/include/uapi/asm-generic/unistd.h
+++ b/include/uapi/asm-generic/unistd.h
@@ -842,8 +842,11 @@ __SYSCALL(__NR_lsm_set_self_attr, sys_lsm_set_self_attr)
#define __NR_lsm_list_modules 461
__SYSCALL(__NR_lsm_list_modules, sys_lsm_list_modules)
+#define __NR_clock_compare 462
+__SYSCALL(__NR_clock_compare, sys_clock_compare)
+
#undef __NR_syscalls
-#define __NR_syscalls 462
+#define __NR_syscalls 463
/*
* 32 bit systems traditionally used different
diff --git a/kernel/time/posix-clock.c b/kernel/time/posix-clock.c
index 9de66bbbb3d1..68b2d6741036 100644
--- a/kernel/time/posix-clock.c
+++ b/kernel/time/posix-clock.c
@@ -327,9 +327,34 @@ static int pc_clock_settime(clockid_t id, const struct timespec64 *ts)
return err;
}
+static int pc_clock_get_crosstime(clockid_t id, struct system_device_crosststamp *xtstamp)
+{
+ struct posix_clock_desc cd;
+ int err;
+
+ err = get_clock_desc(id, &cd);
+ if (err)
+ return err;
+
+ if ((cd.fp->f_mode & FMODE_WRITE) == 0) {
+ err = -EACCES;
+ goto out;
+ }
+
+ if (cd.clk->ops.clock_getcrosstime)
+ err = cd.clk->ops.clock_getcrosstime(cd.clk, xtstamp);
+ else
+ err = -EOPNOTSUPP;
+out:
+ put_clock_desc(&cd);
+
+ return err;
+}
+
const struct k_clock clock_posix_dynamic = {
.clock_getres = pc_clock_getres,
.clock_set = pc_clock_settime,
.clock_get_timespec = pc_clock_gettime,
.clock_adj = pc_clock_adjtime,
+ .clock_get_crosstimespec = pc_clock_get_crosstime,
};
diff --git a/kernel/time/posix-timers.c b/kernel/time/posix-timers.c
index b924f0f096fa..ed082664774b 100644
--- a/kernel/time/posix-timers.c
+++ b/kernel/time/posix-timers.c
@@ -1426,6 +1426,151 @@ SYSCALL_DEFINE4(clock_nanosleep_time32, clockid_t, which_clock, int, flags,
#endif
+/**
+ * clock_compare - Get couple of clocks time stamps
+ * @clock_a: clock a ID
+ * @clock_b: clock b ID
+ * @tp_a: Pointer to a user space timespec64 for clock a storage
+ * @tp_b: Pointer to a user space timespec64 for clock b storage
+ *
+ * clock_compare gets time sample of two clocks.
+ * Supported clocks IDs: PHC, virtual PHC and various system clocks.
+ *
+ * In case of PHC that supports crosstimespec and the other clock is Monotonic raw
+ * or system time, crosstimespec will be used to synchronously capture
+ * system/device time stamp.
+ *
+ * In other cases: Read clock_a twice (before, and after reading clock_b) and
+ * average these times – to be as close as possible to the time we read clock_b.
+ *
+ * Returns:
+ * 0 Success. @tp_a and @tp_b contains the time stamps
+ * -EINVAL @clock a or b ID is not a valid clock ID
+ * -EFAULT Copying the time stamp to @tp_a or @tp_b faulted
+ * -EOPNOTSUPP Dynamic POSIX clock does not support crosstimespec()
+ **/
+SYSCALL_DEFINE5(clock_compare, const clockid_t, clock_a, const clockid_t, clock_b,
+ struct __kernel_timespec __user *, tp_a, struct __kernel_timespec __user *,
+ tp_b, int64_t __user *, offs_err)
+{
+ struct timespec64 ts_a1, ts_b, ts_a2;
+ struct system_device_crosststamp xtstamp_a1, xtstamp_a2, xtstamp_b;
+ const struct k_clock *kc_a, *kc_b;
+ ktime_t ktime_a, ktime_a1, ktime_a2;
+ s64 ts_offs, ts_offs_err = 0;
+ int error = 0;
+ bool crosstime_support_a = false;
+ bool crosstime_support_b = false;
+
+ kc_a = clockid_to_kclock(clock_a);
+ if (!kc_a) {
+ error = -EINVAL;
+ return error;
+ }
+
+ kc_b = clockid_to_kclock(clock_b);
+ if (!kc_b) {
+ error = -EINVAL;
+ return error;
+ }
+
+ // In case crosstimespec supported and b clock is Monotonic raw or system
+ // time, synchronously capture system/device time stamp
+ if (clock_a < 0) {
+ error = kc_a->clock_get_crosstimespec(clock_a, &xtstamp_a1);
+ if (!error) {
+ if (clock_b == CLOCK_MONOTONIC_RAW) {
+ ts_b = ktime_to_timespec64(xtstamp_a1.sys_monoraw);
+ ts_a1 = ktime_to_timespec64(xtstamp_a1.device);
+ goto out;
+ } else if (clock_b == CLOCK_REALTIME) {
+ ts_b = ktime_to_timespec64(xtstamp_a1.sys_realtime);
+ ts_a1 = ktime_to_timespec64(xtstamp_a1.device);
+ goto out;
+ } else {
+ crosstime_support_a = true;
+ }
+ }
+ }
+
+ // In case crosstimespec supported and a clock is Monotonic raw or system
+ // time, synchronously capture system/device time stamp
+ if (clock_b < 0) {
+ // Synchronously capture system/device time stamp
+ error = kc_b->clock_get_crosstimespec(clock_b, &xtstamp_b);
+ if (!error) {
+ if (clock_a == CLOCK_MONOTONIC_RAW) {
+ ts_a1 = ktime_to_timespec64(xtstamp_b.sys_monoraw);
+ ts_b = ktime_to_timespec64(xtstamp_b.device);
+ goto out;
+ } else if (clock_a == CLOCK_REALTIME) {
+ ts_a1 = ktime_to_timespec64(xtstamp_b.sys_realtime);
+ ts_b = ktime_to_timespec64(xtstamp_b.device);
+ goto out;
+ } else {
+ crosstime_support_b = true;
+ }
+ }
+ }
+
+ if (crosstime_support_a)
+ error = kc_a->clock_get_crosstimespec(clock_a, &xtstamp_a1);
+ else
+ error = kc_a->clock_get_timespec(clock_a, &ts_a1);
+
+ if (error)
+ return error;
+
+ if (crosstime_support_b)
+ error = kc_b->clock_get_crosstimespec(clock_b, &xtstamp_b);
+ else
+ error = kc_b->clock_get_timespec(clock_b, &ts_b);
+
+ if (error)
+ return error;
+
+ if (crosstime_support_a)
+ error = kc_a->clock_get_crosstimespec(clock_a, &xtstamp_a2);
+ else
+ error = kc_a->clock_get_timespec(clock_a, &ts_a2);
+
+ if (error)
+ return error;
+
+ if (crosstime_support_a) {
+ ktime_a1 = xtstamp_a1.device;
+ ktime_a2 = xtstamp_a2.device;
+ } else {
+ ktime_a1 = timespec64_to_ktime(ts_a1);
+ ktime_a2 = timespec64_to_ktime(ts_a2);
+ }
+
+ ktime_a = ktime_add(ktime_a1, ktime_a2);
+
+ ts_offs = ktime_divns(ktime_a, 2);
+
+ ts_a1 = ns_to_timespec64(ts_offs);
+
+ ktime_a = ktime_sub(ktime_a2, ktime_a1);
+
+ ts_offs_err = ktime_divns(ktime_a, 2);
+
+ if (crosstime_support_b)
+ ts_b = ktime_to_timespec64(xtstamp_a2.device);
+
+out:
+ if (put_timespec64(&ts_a1, tp_a))
+ error = -EFAULT;
+
+ if (!error && put_timespec64(&ts_b, tp_b))
+ error = -EFAULT;
+
+ if (!error && copy_to_user(offs_err, &ts_offs_err, sizeof(ts_offs_err)))
+ error = -EFAULT;
+
+ return error;
+}
+
static const struct k_clock clock_realtime = {
.clock_getres = posix_get_hrtimer_res,
.clock_get_timespec = posix_get_realtime_timespec,
diff --git a/kernel/time/posix-timers.h b/kernel/time/posix-timers.h
index f32a2ebba9b8..b1f6075f35bb 100644
--- a/kernel/time/posix-timers.h
+++ b/kernel/time/posix-timers.h
@@ -11,6 +11,8 @@ struct k_clock {
struct timespec64 *tp);
/* Returns the clock value in the root time namespace. */
ktime_t (*clock_get_ktime)(const clockid_t which_clock);
+ int (*clock_get_crosstimespec)(const clockid_t which_clock,
+ struct system_device_crosststamp *xtstamp);
int (*clock_adj)(const clockid_t which_clock, struct __kernel_timex *tx);
int (*timer_create)(struct k_itimer *timer);
int (*nsleep)(const clockid_t which_clock, int flags,
--
2.26.3
^ permalink raw reply related
* Re: [PATCH v15 05/11] LSM: Create lsm_list_modules system call
From: Dmitry V. Levin @ 2024-03-12 10:16 UTC (permalink / raw)
To: Casey Schaufler
Cc: paul, linux-security-module, jmorris, serge, keescook,
john.johansen, penguin-kernel, stephen.smalley.work, linux-kernel,
linux-api, mic
In-Reply-To: <20230912205658.3432-6-casey@schaufler-ca.com>
Hi,
On Tue, Sep 12, 2023 at 01:56:50PM -0700, Casey Schaufler wrote:
[...]
> --- a/security/lsm_syscalls.c
> +++ b/security/lsm_syscalls.c
> @@ -55,3 +55,42 @@ SYSCALL_DEFINE4(lsm_get_self_attr, unsigned int, attr, struct lsm_ctx __user *,
> {
> return security_getselfattr(attr, ctx, size, flags);
> }
> +
> +/**
> + * sys_lsm_list_modules - Return a list of the active security modules
> + * @ids: the LSM module ids
> + * @size: pointer to size of @ids, updated on return
> + * @flags: reserved for future use, must be zero
> + *
> + * Returns a list of the active LSM ids. On success this function
> + * returns the number of @ids array elements. This value may be zero
> + * if there are no LSMs active. If @size is insufficient to contain
> + * the return data -E2BIG is returned and @size is set to the minimum
> + * required size. In all other cases a negative value indicating the
> + * error is returned.
> + */
> +SYSCALL_DEFINE3(lsm_list_modules, u64 __user *, ids, size_t __user *, size,
> + u32, flags)
I'm sorry but the size of userspace size_t is different from the kernel one
on 32-bit compat architectures.
Looks like there has to be a COMPAT_SYSCALL_DEFINE3(lsm_list_modules, ...)
now. Other two added lsm syscalls also have this issue.
--
ldv
^ permalink raw reply
* Re: [PATCH v6] posix-timers: add clock_compare system call
From: Arnd Bergmann @ 2024-03-12 11:17 UTC (permalink / raw)
To: Sagi Maimon, Richard Cochran, Andy Lutomirski, datglx,
Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
Geert Uytterhoeven, Peter Zijlstra, Johannes Weiner, Sohil Mehta,
Rick Edgecombe, Nhat Pham, Palmer Dabbelt, Kees Cook,
Alexey Gladkov, Mark Rutland, Miklos Szeredi, Casey Schaufler,
reibax, David S . Miller, Christian Brauner
Cc: linux-kernel, linux-api, Linux-Arch, Netdev
In-Reply-To: <20240312095005.8909-1-maimon.sagi@gmail.com>
On Tue, Mar 12, 2024, at 10:50, Sagi Maimon wrote:
> Some user space applications need to read a couple of different clocks.
> Each read requires moving from user space to kernel space.
> Reading each clock separately (syscall) introduces extra
> unpredictable/unmeasurable delay. Minimizing this delay contributes to user
> space actions on these clocks (e.g. synchronization etc).
>
> Introduce a new system call clock_compare, which can be used to measure
> the offset between two clocks, from variety of types: PHC, virtual PHC
> and various system clocks (CLOCK_REALTIME, CLOCK_MONOTONIC, etc).
> The system call returns the clocks timestamps.
>
> When possible, use crosstimespec to sync read values.
> Else, read clock A twice (before, and after reading clock B) and average these
> times – to be as close as possible to the time we read clock B.
>
> Signed-off-by: Sagi Maimon <maimon.sagi@gmail.com>
I like this a lot better than the previous versions I looked at,
so just a few ideas here how this might be improved further.
> +/**
> + * clock_compare - Get couple of clocks time stamps
> + * @clock_a: clock a ID
> + * @clock_b: clock b ID
> + * @tp_a: Pointer to a user space timespec64 for clock a storage
> + * @tp_b: Pointer to a user space timespec64 for clock b storage
> + *
> + * clock_compare gets time sample of two clocks.
> + * Supported clocks IDs: PHC, virtual PHC and various system clocks.
> + *
> + * In case of PHC that supports crosstimespec and the other clock is
> Monotonic raw
> + * or system time, crosstimespec will be used to synchronously capture
> + * system/device time stamp.
> + *
> + * In other cases: Read clock_a twice (before, and after reading
> clock_b) and
> + * average these times – to be as close as possible to the time we
> read clock_b.
> + *
> + * Returns:
> + * 0 Success. @tp_a and @tp_b contains the time stamps
> + * -EINVAL @clock a or b ID is not a valid clock ID
> + * -EFAULT Copying the time stamp to @tp_a or @tp_b faulted
> + * -EOPNOTSUPP Dynamic POSIX clock does not support crosstimespec()
> + **/
> +SYSCALL_DEFINE5(clock_compare, const clockid_t, clock_a, const
> clockid_t, clock_b,
> + struct __kernel_timespec __user *, tp_a, struct __kernel_timespec
> __user *,
> + tp_b, int64_t __user *, offs_err)
The system call is well-formed in the way that the ABI is the
same across all supported architectures, good.
A minor issue is the use of int64_t, which in user interfaces
can cause namespace problems. Please change that to the kernel
side __s64 type.
> + kc_a = clockid_to_kclock(clock_a);
> + if (!kc_a) {
> + error = -EINVAL;
> + return error;
> + }
> +
> + kc_b = clockid_to_kclock(clock_b);
> + if (!kc_b) {
> + error = -EINVAL;
> + return error;
> + }
I'm not sure if we really need to have it generic enough to
support any combination of clocks here. It complicates the
implementation a bit but it also generalizes the user space
side of it.
Can you think of cases where you want to compare against
something other than CLOCK_MONOTONIC_RAW or CLOCK_REALTIME,
or are these going to be the ones that you expect to
be used anyway?
> + if (crosstime_support_a) {
> + ktime_a1 = xtstamp_a1.device;
> + ktime_a2 = xtstamp_a2.device;
> + } else {
> + ktime_a1 = timespec64_to_ktime(ts_a1);
> + ktime_a2 = timespec64_to_ktime(ts_a2);
> + }
> +
> + ktime_a = ktime_add(ktime_a1, ktime_a2);
> +
> + ts_offs = ktime_divns(ktime_a, 2);
> +
> + ts_a1 = ns_to_timespec64(ts_offs);
Converting nanoseconds to timespec64 is rather expensive,
so I wonder if this could be changed to something cheaper,
either by returning nanoseconds in the end and consistently
working on those, or by doing the calculation on the
timespec64 itself.
Arnd
^ permalink raw reply
* Re: [PATCH v6] posix-timers: add clock_compare system call
From: Sagi Maimon @ 2024-03-12 12:15 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Richard Cochran, Andy Lutomirski, datglx, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
Geert Uytterhoeven, Peter Zijlstra, Johannes Weiner, Sohil Mehta,
Rick Edgecombe, Nhat Pham, Palmer Dabbelt, Kees Cook,
Alexey Gladkov, Mark Rutland, Miklos Szeredi, Casey Schaufler,
reibax, David S . Miller, Christian Brauner, linux-kernel,
linux-api, Linux-Arch, Netdev
In-Reply-To: <7bf7d444-4a08-4df4-9aa1-9cd28609d166@app.fastmail.com>
Hi Arnd
Thanks for you comments.
On Tue, Mar 12, 2024 at 1:19 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Tue, Mar 12, 2024, at 10:50, Sagi Maimon wrote:
> > Some user space applications need to read a couple of different clocks.
> > Each read requires moving from user space to kernel space.
> > Reading each clock separately (syscall) introduces extra
> > unpredictable/unmeasurable delay. Minimizing this delay contributes to user
> > space actions on these clocks (e.g. synchronization etc).
> >
> > Introduce a new system call clock_compare, which can be used to measure
> > the offset between two clocks, from variety of types: PHC, virtual PHC
> > and various system clocks (CLOCK_REALTIME, CLOCK_MONOTONIC, etc).
> > The system call returns the clocks timestamps.
> >
> > When possible, use crosstimespec to sync read values.
> > Else, read clock A twice (before, and after reading clock B) and average these
> > times – to be as close as possible to the time we read clock B.
> >
> > Signed-off-by: Sagi Maimon <maimon.sagi@gmail.com>
>
> I like this a lot better than the previous versions I looked at,
> so just a few ideas here how this might be improved further.
>
> > +/**
> > + * clock_compare - Get couple of clocks time stamps
> > + * @clock_a: clock a ID
> > + * @clock_b: clock b ID
> > + * @tp_a: Pointer to a user space timespec64 for clock a storage
> > + * @tp_b: Pointer to a user space timespec64 for clock b storage
> > + *
> > + * clock_compare gets time sample of two clocks.
> > + * Supported clocks IDs: PHC, virtual PHC and various system clocks.
> > + *
> > + * In case of PHC that supports crosstimespec and the other clock is
> > Monotonic raw
> > + * or system time, crosstimespec will be used to synchronously capture
> > + * system/device time stamp.
> > + *
> > + * In other cases: Read clock_a twice (before, and after reading
> > clock_b) and
> > + * average these times – to be as close as possible to the time we
> > read clock_b.
> > + *
> > + * Returns:
> > + * 0 Success. @tp_a and @tp_b contains the time stamps
> > + * -EINVAL @clock a or b ID is not a valid clock ID
> > + * -EFAULT Copying the time stamp to @tp_a or @tp_b faulted
> > + * -EOPNOTSUPP Dynamic POSIX clock does not support crosstimespec()
> > + **/
> > +SYSCALL_DEFINE5(clock_compare, const clockid_t, clock_a, const
> > clockid_t, clock_b,
> > + struct __kernel_timespec __user *, tp_a, struct __kernel_timespec
> > __user *,
> > + tp_b, int64_t __user *, offs_err)
>
> The system call is well-formed in the way that the ABI is the
> same across all supported architectures, good.
>
> A minor issue is the use of int64_t, which in user interfaces
> can cause namespace problems. Please change that to the kernel
> side __s64 type.
>
you are right - it will be fixed on the next patch
> > + kc_a = clockid_to_kclock(clock_a);
> > + if (!kc_a) {
> > + error = -EINVAL;
> > + return error;
> > + }
> > +
> > + kc_b = clockid_to_kclock(clock_b);
> > + if (!kc_b) {
> > + error = -EINVAL;
> > + return error;
> > + }
>
> I'm not sure if we really need to have it generic enough to
> support any combination of clocks here. It complicates the
> implementation a bit but it also generalizes the user space
> side of it.
>
> Can you think of cases where you want to compare against
> something other than CLOCK_MONOTONIC_RAW or CLOCK_REALTIME,
> or are these going to be the ones that you expect to
> be used anyway?
>
sure, one example is syncing two different PHCs (which was originally
why we needed this syscall)
I hope that I have understand your note and that answers your question.
> > + if (crosstime_support_a) {
> > + ktime_a1 = xtstamp_a1.device;
> > + ktime_a2 = xtstamp_a2.device;
> > + } else {
> > + ktime_a1 = timespec64_to_ktime(ts_a1);
> > + ktime_a2 = timespec64_to_ktime(ts_a2);
> > + }
> > +
> > + ktime_a = ktime_add(ktime_a1, ktime_a2);
> > +
> > + ts_offs = ktime_divns(ktime_a, 2);
> > +
> > + ts_a1 = ns_to_timespec64(ts_offs);
>
> Converting nanoseconds to timespec64 is rather expensive,
> so I wonder if this could be changed to something cheaper,
> either by returning nanoseconds in the end and consistently
> working on those, or by doing the calculation on the
> timespec64 itself.
>
I prefer returning timespec64, so this system call aligns with other
system calls like clock_gettime for example.
As far as doing the calculation on timespec64 itself, that looks more
expansive to me, but I might be wrong.
Sagi
> Arnd
^ permalink raw reply
* Re: [PATCH v15 05/11] LSM: Create lsm_list_modules system call
From: Paul Moore @ 2024-03-12 13:25 UTC (permalink / raw)
To: Dmitry V. Levin
Cc: Casey Schaufler, linux-security-module, jmorris, serge, keescook,
john.johansen, penguin-kernel, stephen.smalley.work, linux-kernel,
linux-api, mic
In-Reply-To: <20240312101630.GA903@altlinux.org>
On Tue, Mar 12, 2024 at 6:16 AM Dmitry V. Levin <ldv@strace.io> wrote:
> On Tue, Sep 12, 2023 at 01:56:50PM -0700, Casey Schaufler wrote:
> [...]
> > --- a/security/lsm_syscalls.c
> > +++ b/security/lsm_syscalls.c
> > @@ -55,3 +55,42 @@ SYSCALL_DEFINE4(lsm_get_self_attr, unsigned int, attr, struct lsm_ctx __user *,
> > {
> > return security_getselfattr(attr, ctx, size, flags);
> > }
> > +
> > +/**
> > + * sys_lsm_list_modules - Return a list of the active security modules
> > + * @ids: the LSM module ids
> > + * @size: pointer to size of @ids, updated on return
> > + * @flags: reserved for future use, must be zero
> > + *
> > + * Returns a list of the active LSM ids. On success this function
> > + * returns the number of @ids array elements. This value may be zero
> > + * if there are no LSMs active. If @size is insufficient to contain
> > + * the return data -E2BIG is returned and @size is set to the minimum
> > + * required size. In all other cases a negative value indicating the
> > + * error is returned.
> > + */
> > +SYSCALL_DEFINE3(lsm_list_modules, u64 __user *, ids, size_t __user *, size,
> > + u32, flags)
>
> I'm sorry but the size of userspace size_t is different from the kernel one
> on 32-bit compat architectures.
D'oh, yes, thanks for pointing that out. It would have been nice to
have caught that before v6.8 was released, but I guess it's better
than later.
> Looks like there has to be a COMPAT_SYSCALL_DEFINE3(lsm_list_modules, ...)
> now. Other two added lsm syscalls also have this issue.
Considering that Linux v6.8, and by extension these syscalls, are only
a few days old, I think I'd rather see us just modify the syscalls and
avoid the compat baggage. I'm going to be shocked if anyone has
shifted to using the new syscalls yet, and even if they have (!!),
moving from a "size_t" type to a "u64" should be mostly transparent
for the majority of native 64-bit systems. Those running the absolute
latest kernels on 32-bit systems with custom or bleeding edge
userspace *may* see a slight hiccup, but I think that user count is in
the single digits, if not zero.
Let's fix this quickly with /size_t/u64/ in v6.8.1 and avoid the
compat shim if we can.
Casey, do you have time to put together a patch for this (you should
fix the call chains below the syscalls too)? If not, please let me
know and I'll get a patch out ASAP.
Thanks all.
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH v6] posix-timers: add clock_compare system call
From: Arnd Bergmann @ 2024-03-12 13:47 UTC (permalink / raw)
To: Sagi Maimon
Cc: Richard Cochran, Andy Lutomirski, datglx, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
Geert Uytterhoeven, Peter Zijlstra, Johannes Weiner, Sohil Mehta,
Rick Edgecombe, Nhat Pham, Palmer Dabbelt, Kees Cook,
Alexey Gladkov, Mark Rutland, Miklos Szeredi, Casey Schaufler,
reibax, David S . Miller, Christian Brauner, linux-kernel,
linux-api, Linux-Arch, Netdev
In-Reply-To: <CAMuE1bGkZ=ifyofCUfm4JVS__dgYG41kecS4TxBaHJvyJ607PQ@mail.gmail.com>
On Tue, Mar 12, 2024, at 13:15, Sagi Maimon wrote:
> On Tue, Mar 12, 2024 at 1:19 PM Arnd Bergmann <arnd@arndb.de> wrote:
>> On Tue, Mar 12, 2024, at 10:50, Sagi Maimon wrote:
>> > + kc_a = clockid_to_kclock(clock_a);
>> > + if (!kc_a) {
>> > + error = -EINVAL;
>> > + return error;
>> > + }
>> > +
>> > + kc_b = clockid_to_kclock(clock_b);
>> > + if (!kc_b) {
>> > + error = -EINVAL;
>> > + return error;
>> > + }
>>
>> I'm not sure if we really need to have it generic enough to
>> support any combination of clocks here. It complicates the
>> implementation a bit but it also generalizes the user space
>> side of it.
>>
>> Can you think of cases where you want to compare against
>> something other than CLOCK_MONOTONIC_RAW or CLOCK_REALTIME,
>> or are these going to be the ones that you expect to
>> be used anyway?
>>
> sure, one example is syncing two different PHCs (which was originally
> why we needed this syscall)
> I hope that I have understand your note and that answers your question.
Right, that is clearly a sensible use case.
I'm still trying to understand the implementation for the case
where you have two different PHCs and both implement
clock_get_crosstimespec(). Rather than averaging between
two snapshots here, I would expect this to result in
something like
ktime_a1 += xtstamp_b.sys_monoraw - xtstamp_a1.sys_monoraw;
in order get two device timestamps ktime_a1 and ktime_b
that reflect the snapshots as if they were taken
simulatenously. Am I missing some finer detail here,
or is this something you should do?
>> > + if (crosstime_support_a) {
>> > + ktime_a1 = xtstamp_a1.device;
>> > + ktime_a2 = xtstamp_a2.device;
>> > + } else {
>> > + ktime_a1 = timespec64_to_ktime(ts_a1);
>> > + ktime_a2 = timespec64_to_ktime(ts_a2);
>> > + }
>> > +
>> > + ktime_a = ktime_add(ktime_a1, ktime_a2);
>> > +
>> > + ts_offs = ktime_divns(ktime_a, 2);
>> > +
>> > + ts_a1 = ns_to_timespec64(ts_offs);
>>
>> Converting nanoseconds to timespec64 is rather expensive,
>> so I wonder if this could be changed to something cheaper,
>> either by returning nanoseconds in the end and consistently
>> working on those, or by doing the calculation on the
>> timespec64 itself.
>>
> I prefer returning timespec64, so this system call aligns with other
> system calls like clock_gettime for example.
> As far as doing the calculation on timespec64 itself, that looks more
> expansive to me, but I might be wrong.
In the general case, dividing a 64-bit variable by some other
variable is really expensive and will take hundreds of cycles.
This one is a bit cheaper because the division is done using
a constant divider of NS_PER_SEC, which can get optimized fairly
well on many systems by turning it into an equivalent 128-bit
multiplication plus shift.
For the case where you start out with a timespec64, I would
expect it to be cheaper to calculate the nanosecond difference
between ts_a1 and ts_a2 to add half of that to the timespec
than to average two large 64-bit values and convert that back
to a timespec afterwards. This should be fairly easy to try
out if you can test a 32-bit kernel. We could decide that
there is no need to care about anything bug 64-bit kernels
here, in which case your current version should be just as
good for both the crosstime_support_a and !crosstime_support_a
cases.
Arnd
^ permalink raw reply
* [PATCH v3 0/2] VT: Add ability to get font requirements
From: legion @ 2024-03-12 14:23 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby
Cc: Alexey Gladkov, LKML, kbd, linux-api, linux-fbdev, linux-serial
In-Reply-To: <cover.1708960303.git.legion@kernel.org>
From: Alexey Gladkov <legion@kernel.org>
We now have KD_FONT_OP_SET_TALL, but in fact such large fonts cannot be
loaded. No console driver supports tall fonts. Unfortunately, userspace
cannot distinguish the lack of support in the driver from errors in the
font itself. In all cases, EINVAL will be returned.
This patchset adds a separate ioctl to obtain the font parameters
supported by the console driver.
v3:
* Added the use of the in_range macro.
* Squashed the commits that add ioctl to console divers.
v2:
* Instead of the KDFONTOP extension, a new ioctl has been added to
obtain font information.
---
Alexey Gladkov (2):
VT: Add KDFONTINFO ioctl
VT: Allow to get max font width and height
drivers/tty/vt/vt.c | 24 ++++++++++++++++++++++++
drivers/tty/vt/vt_ioctl.c | 13 +++++++++++++
drivers/video/console/newport_con.c | 21 +++++++++++++++++----
drivers/video/console/sticon.c | 25 +++++++++++++++++++++++--
drivers/video/console/vgacon.c | 21 ++++++++++++++++++++-
drivers/video/fbdev/core/fbcon.c | 22 +++++++++++++++++++++-
include/linux/console.h | 2 ++
include/linux/vt_kern.h | 1 +
include/uapi/linux/kd.h | 13 ++++++++++++-
9 files changed, 133 insertions(+), 9 deletions(-)
--
2.44.0
^ permalink raw reply
* [PATCH v3 1/2] VT: Add KDFONTINFO ioctl
From: legion @ 2024-03-12 14:23 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby
Cc: Alexey Gladkov, LKML, kbd, linux-api, linux-fbdev, linux-serial
In-Reply-To: <cover.1710252966.git.legion@kernel.org>
From: Alexey Gladkov <legion@kernel.org>
Each driver has its own restrictions on font size. There is currently no
way to understand what the requirements are. The new ioctl allows
userspace to get the minmum and maximum font size values.
Signed-off-by: Alexey Gladkov <legion@kernel.org>
---
drivers/tty/vt/vt.c | 24 ++++++++++++++++++++++++
drivers/tty/vt/vt_ioctl.c | 13 +++++++++++++
include/linux/console.h | 2 ++
include/linux/vt_kern.h | 1 +
include/uapi/linux/kd.h | 13 ++++++++++++-
5 files changed, 52 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 156efda7c80d..8c2a3d98b5ec 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -4680,6 +4680,30 @@ int con_font_op(struct vc_data *vc, struct console_font_op *op)
return -ENOSYS;
}
+int con_font_info(struct vc_data *vc, struct console_font_info *info)
+{
+ int rc = -EINVAL;
+
+ info->min_height = 0;
+ info->max_height = max_font_height;
+
+ info->min_width = 0;
+ info->max_width = max_font_width;
+
+ info->flags = KD_FONT_INFO_FLAG_LOW_SIZE | KD_FONT_INFO_FLAG_HIGH_SIZE;
+
+ console_lock();
+ if (vc->vc_mode != KD_TEXT)
+ rc = -EINVAL;
+ else if (vc->vc_sw->con_font_info)
+ rc = vc->vc_sw->con_font_info(vc, info);
+ else
+ rc = -ENOSYS;
+ console_unlock();
+
+ return rc;
+}
+
/*
* Interface exported to selection and vcs.
*/
diff --git a/drivers/tty/vt/vt_ioctl.c b/drivers/tty/vt/vt_ioctl.c
index 8c685b501404..b3b4e4b69366 100644
--- a/drivers/tty/vt/vt_ioctl.c
+++ b/drivers/tty/vt/vt_ioctl.c
@@ -479,6 +479,19 @@ static int vt_k_ioctl(struct tty_struct *tty, unsigned int cmd,
break;
}
+ case KDFONTINFO: {
+ struct console_font_info fnt_info;
+
+ if (copy_from_user(&fnt_info, up, sizeof(fnt_info)))
+ return -EFAULT;
+ ret = con_font_info(vc, &fnt_info);
+ if (ret)
+ return ret;
+ if (copy_to_user(up, &fnt_info, sizeof(fnt_info)))
+ return -EFAULT;
+ break;
+ }
+
default:
return -ENOIOCTLCMD;
}
diff --git a/include/linux/console.h b/include/linux/console.h
index 779d388af8a0..5bea6f6c2042 100644
--- a/include/linux/console.h
+++ b/include/linux/console.h
@@ -20,6 +20,7 @@
#include <linux/types.h>
struct vc_data;
+struct console_font_info;
struct console_font_op;
struct console_font;
struct module;
@@ -59,6 +60,7 @@ struct consw {
unsigned int lines);
int (*con_switch)(struct vc_data *vc);
int (*con_blank)(struct vc_data *vc, int blank, int mode_switch);
+ int (*con_font_info)(struct vc_data *vc, struct console_font_info *info);
int (*con_font_set)(struct vc_data *vc, struct console_font *font,
unsigned int vpitch, unsigned int flags);
int (*con_font_get)(struct vc_data *vc, struct console_font *font,
diff --git a/include/linux/vt_kern.h b/include/linux/vt_kern.h
index c1f5aebef170..6bda4cc1fe6f 100644
--- a/include/linux/vt_kern.h
+++ b/include/linux/vt_kern.h
@@ -32,6 +32,7 @@ void do_blank_screen(int entering_gfx);
void do_unblank_screen(int leaving_gfx);
void poke_blanked_console(void);
int con_font_op(struct vc_data *vc, struct console_font_op *op);
+int con_font_info(struct vc_data *vc, struct console_font_info *info);
int con_set_cmap(unsigned char __user *cmap);
int con_get_cmap(unsigned char __user *cmap);
void scrollback(struct vc_data *vc);
diff --git a/include/uapi/linux/kd.h b/include/uapi/linux/kd.h
index 6b384065c013..781e086e55bf 100644
--- a/include/uapi/linux/kd.h
+++ b/include/uapi/linux/kd.h
@@ -183,8 +183,19 @@ struct console_font {
#define KD_FONT_FLAG_DONT_RECALC 1 /* Don't recalculate hw charcell size [compat] */
+#define KDFONTINFO 0x4B73 /* font information */
+
+#define KD_FONT_INFO_FLAG_LOW_SIZE (1U << 0) /* 256 */
+#define KD_FONT_INFO_FLAG_HIGH_SIZE (1U << 1) /* 512 */
+
+struct console_font_info {
+ unsigned int min_width, min_height; /* minimal font size */
+ unsigned int max_width, max_height; /* maximum font size */
+ unsigned int flags; /* KD_FONT_INFO_FLAG_* */
+};
+
/* note: 0x4B00-0x4B4E all have had a value at some time;
don't reuse for the time being */
-/* note: 0x4B60-0x4B6D, 0x4B70-0x4B72 used above */
+/* note: 0x4B60-0x4B6D, 0x4B70-0x4B73 used above */
#endif /* _UAPI_LINUX_KD_H */
--
2.44.0
^ permalink raw reply related
* [PATCH v3 2/2] VT: Allow to get max font width and height
From: legion @ 2024-03-12 14:23 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby
Cc: Alexey Gladkov, LKML, kbd, linux-api, linux-fbdev, linux-serial
In-Reply-To: <cover.1710252966.git.legion@kernel.org>
From: Alexey Gladkov <legion@kernel.org>
The Console drivers has more restrictive font size limits than vt_ioctl.
This leads to errors that are difficult to handle. If a font whose size
is not supported is used, an EINVAL error will be returned, which is
also returned in case of errors in the font itself. At the moment there
is no way to understand what font sizes the current console driver
supports.
To solve this problem, we need to transfer information about the
supported font to userspace from the console driver.
Signed-off-by: Alexey Gladkov <legion@kernel.org>
---
drivers/video/console/newport_con.c | 21 +++++++++++++++++----
drivers/video/console/sticon.c | 25 +++++++++++++++++++++++--
drivers/video/console/vgacon.c | 21 ++++++++++++++++++++-
drivers/video/fbdev/core/fbcon.c | 22 +++++++++++++++++++++-
4 files changed, 81 insertions(+), 8 deletions(-)
diff --git a/drivers/video/console/newport_con.c b/drivers/video/console/newport_con.c
index e8e4f82cd4a1..87f174a95fa8 100644
--- a/drivers/video/console/newport_con.c
+++ b/drivers/video/console/newport_con.c
@@ -33,6 +33,9 @@
#define NEWPORT_LEN 0x10000
+#define NEWPORT_MAX_FONT_WIDTH 8
+#define NEWPORT_MAX_FONT_HEIGHT 16
+
#define FONT_DATA ((unsigned char *)font_vga_8x16.data)
static unsigned char *font_data[MAX_NR_CONSOLES];
@@ -328,8 +331,8 @@ static void newport_init(struct vc_data *vc, int init)
{
int cols, rows;
- cols = newport_xsize / 8;
- rows = newport_ysize / 16;
+ cols = newport_xsize / NEWPORT_MAX_FONT_WIDTH;
+ rows = newport_ysize / NEWPORT_MAX_FONT_HEIGHT;
vc->vc_can_do_color = 1;
if (init) {
vc->vc_cols = cols;
@@ -507,8 +510,8 @@ static int newport_set_font(int unit, struct console_font *op, unsigned int vpit
/* ladis: when I grow up, there will be a day... and more sizes will
* be supported ;-) */
- if ((w != 8) || (h != 16) || (vpitch != 32)
- || (op->charcount != 256 && op->charcount != 512))
+ if ((w != NEWPORT_MAX_FONT_WIDTH) || (h != NEWPORT_MAX_FONT_HEIGHT) ||
+ (vpitch != 32) || (op->charcount != 256 && op->charcount != 512))
return -EINVAL;
if (!(new_data = kmalloc(FONT_EXTRA_WORDS * sizeof(int) + size,
@@ -569,6 +572,15 @@ static int newport_font_default(struct vc_data *vc, struct console_font *op, cha
return newport_set_def_font(vc->vc_num, op);
}
+static int newport_font_info(struct vc_data *vc, struct console_font_info *info)
+{
+ info->min_width = info->max_width = NEWPORT_MAX_FONT_WIDTH;
+ info->min_height = info->max_height = NEWPORT_MAX_FONT_HEIGHT;
+ info->flags = KD_FONT_INFO_FLAG_LOW_SIZE | KD_FONT_INFO_FLAG_HIGH_SIZE;
+
+ return 0;
+}
+
static int newport_font_set(struct vc_data *vc, struct console_font *font,
unsigned int vpitch, unsigned int flags)
{
@@ -688,6 +700,7 @@ const struct consw newport_con = {
.con_scroll = newport_scroll,
.con_switch = newport_switch,
.con_blank = newport_blank,
+ .con_font_info = newport_font_info,
.con_font_set = newport_font_set,
.con_font_default = newport_font_default,
.con_save_screen = newport_save_screen
diff --git a/drivers/video/console/sticon.c b/drivers/video/console/sticon.c
index 992a4fa431aa..d32ca458eb77 100644
--- a/drivers/video/console/sticon.c
+++ b/drivers/video/console/sticon.c
@@ -56,6 +56,11 @@
#define BLANK 0
static int vga_is_gfx;
+#define STICON_MIN_FONT_WIDTH 6
+#define STICON_MIN_FONT_HEIGHT 6
+#define STICON_MAX_FONT_WIDTH 32
+#define STICON_MAX_FONT_HEIGHT 32
+
#define STI_DEF_FONT sticon_sti->font
/* borrowed from fbcon.c */
@@ -180,8 +185,10 @@ static int sticon_set_font(struct vc_data *vc, struct console_font *op,
struct sti_cooked_font *cooked_font;
unsigned char *data = op->data, *p;
- if ((w < 6) || (h < 6) || (w > 32) || (h > 32) || (vpitch != 32)
- || (op->charcount != 256 && op->charcount != 512))
+ if (!in_range(w, STICON_MIN_FONT_WIDTH, STICON_MAX_FONT_WIDTH) ||
+ !in_range(h, STICON_MIN_FONT_HEIGHT, STICON_MAX_FONT_HEIGHT) ||
+ (vpitch != 32) ||
+ (op->charcount != 256 && op->charcount != 512))
return -EINVAL;
pitch = ALIGN(w, 8) / 8;
bpc = pitch * h;
@@ -273,6 +280,19 @@ static int sticon_font_set(struct vc_data *vc, struct console_font *font,
return sticon_set_font(vc, font, vpitch);
}
+static int sticon_font_info(struct vc_data *vc, struct console_font_info *info)
+{
+ info->min_width = STICON_MIN_FONT_WIDTH;
+ info->min_height = STICON_MIN_FONT_HEIGHT;
+
+ info->max_width = STICON_MAX_FONT_WIDTH;
+ info->max_height = STICON_MAX_FONT_HEIGHT;
+
+ info->flags = KD_FONT_INFO_FLAG_LOW_SIZE | KD_FONT_INFO_FLAG_HIGH_SIZE;
+
+ return 0;
+}
+
static void sticon_init(struct vc_data *c, int init)
{
struct sti_struct *sti = sticon_sti;
@@ -371,6 +391,7 @@ static const struct consw sti_con = {
.con_scroll = sticon_scroll,
.con_switch = sticon_switch,
.con_blank = sticon_blank,
+ .con_font_info = sticon_font_info,
.con_font_set = sticon_font_set,
.con_font_default = sticon_font_default,
.con_build_attr = sticon_build_attr,
diff --git a/drivers/video/console/vgacon.c b/drivers/video/console/vgacon.c
index 8ef1579fa57f..b75d31ef3353 100644
--- a/drivers/video/console/vgacon.c
+++ b/drivers/video/console/vgacon.c
@@ -61,6 +61,10 @@ static struct vgastate vgastate;
#define BLANK 0x0020
#define VGA_FONTWIDTH 8 /* VGA does not support fontwidths != 8 */
+
+#define VGACON_MAX_FONT_WIDTH VGA_FONTWIDTH
+#define VGACON_MAX_FONT_HEIGHT 32
+
/*
* Interface used by the world
*/
@@ -1013,6 +1017,19 @@ static int vgacon_adjust_height(struct vc_data *vc, unsigned fontheight)
return 0;
}
+static int vgacon_font_info(struct vc_data *vc, struct console_font_info *info)
+{
+ info->min_width = VGACON_MAX_FONT_WIDTH;
+ info->min_height = 0;
+
+ info->max_width = VGACON_MAX_FONT_WIDTH;
+ info->max_height = VGACON_MAX_FONT_HEIGHT;
+
+ info->flags = KD_FONT_INFO_FLAG_LOW_SIZE | KD_FONT_INFO_FLAG_HIGH_SIZE;
+
+ return 0;
+}
+
static int vgacon_font_set(struct vc_data *c, struct console_font *font,
unsigned int vpitch, unsigned int flags)
{
@@ -1022,7 +1039,8 @@ static int vgacon_font_set(struct vc_data *c, struct console_font *font,
if (vga_video_type < VIDEO_TYPE_EGAM)
return -EINVAL;
- if (font->width != VGA_FONTWIDTH || font->height > 32 || vpitch != 32 ||
+ if (font->width != VGACON_MAX_FONT_WIDTH ||
+ font->height > VGACON_MAX_FONT_HEIGHT || vpitch != 32 ||
(charcount != 256 && charcount != 512))
return -EINVAL;
@@ -1177,6 +1195,7 @@ const struct consw vga_con = {
.con_scroll = vgacon_scroll,
.con_switch = vgacon_switch,
.con_blank = vgacon_blank,
+ .con_font_info = vgacon_font_info,
.con_font_set = vgacon_font_set,
.con_font_get = vgacon_font_get,
.con_resize = vgacon_resize,
diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index 46823c2e2ba1..e10abe416159 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -101,6 +101,9 @@ enum {
FBCON_LOGO_DONTSHOW = -3 /* do not show the logo */
};
+#define FBCON_MAX_FONT_WIDTH 32
+#define FBCON_MAX_FONT_HEIGHT 32
+
static struct fbcon_display fb_display[MAX_NR_CONSOLES];
static struct fb_info *fbcon_registered_fb[FB_MAX];
@@ -2456,6 +2459,21 @@ static int fbcon_do_set_font(struct vc_data *vc, int w, int h, int charcount,
return ret;
}
+
+static int fbcon_font_info(struct vc_data *vc, struct console_font_info *info)
+{
+ info->min_width = 0;
+ info->min_height = 0;
+
+ info->max_width = FBCON_MAX_FONT_WIDTH;
+ info->max_height = FBCON_MAX_FONT_HEIGHT;
+
+ info->flags = KD_FONT_INFO_FLAG_LOW_SIZE | KD_FONT_INFO_FLAG_HIGH_SIZE;
+
+ return 0;
+}
+
+
/*
* User asked to set font; we are guaranteed that charcount does not exceed 512
* but lets not assume that, since charcount of 512 is small for unicode support.
@@ -2483,7 +2501,8 @@ static int fbcon_set_font(struct vc_data *vc, struct console_font *font,
h > FBCON_SWAP(info->var.rotate, info->var.yres, info->var.xres))
return -EINVAL;
- if (font->width > 32 || font->height > 32)
+ if (font->width > FBCON_MAX_FONT_WIDTH ||
+ font->height > FBCON_MAX_FONT_HEIGHT)
return -EINVAL;
/* Make sure drawing engine can handle the font */
@@ -3158,6 +3177,7 @@ static const struct consw fb_con = {
.con_scroll = fbcon_scroll,
.con_switch = fbcon_switch,
.con_blank = fbcon_blank,
+ .con_font_info = fbcon_font_info,
.con_font_set = fbcon_set_font,
.con_font_get = fbcon_get_font,
.con_font_default = fbcon_set_def_font,
--
2.44.0
^ permalink raw reply related
* Re: [PATCH v15 05/11] LSM: Create lsm_list_modules system call
From: Casey Schaufler @ 2024-03-12 15:27 UTC (permalink / raw)
To: Paul Moore, Dmitry V. Levin
Cc: linux-security-module, jmorris, serge, keescook, john.johansen,
penguin-kernel, stephen.smalley.work, linux-kernel, linux-api,
mic, Casey Schaufler
In-Reply-To: <CAHC9VhRgjNT2YnVgCqMJnyr227qUjmfrWZ+LBnu_DGxnJZgeKw@mail.gmail.com>
On 3/12/2024 6:25 AM, Paul Moore wrote:
> On Tue, Mar 12, 2024 at 6:16 AM Dmitry V. Levin <ldv@strace.io> wrote:
>> On Tue, Sep 12, 2023 at 01:56:50PM -0700, Casey Schaufler wrote:
>> [...]
>>> --- a/security/lsm_syscalls.c
>>> +++ b/security/lsm_syscalls.c
>>> @@ -55,3 +55,42 @@ SYSCALL_DEFINE4(lsm_get_self_attr, unsigned int, attr, struct lsm_ctx __user *,
>>> {
>>> return security_getselfattr(attr, ctx, size, flags);
>>> }
>>> +
>>> +/**
>>> + * sys_lsm_list_modules - Return a list of the active security modules
>>> + * @ids: the LSM module ids
>>> + * @size: pointer to size of @ids, updated on return
>>> + * @flags: reserved for future use, must be zero
>>> + *
>>> + * Returns a list of the active LSM ids. On success this function
>>> + * returns the number of @ids array elements. This value may be zero
>>> + * if there are no LSMs active. If @size is insufficient to contain
>>> + * the return data -E2BIG is returned and @size is set to the minimum
>>> + * required size. In all other cases a negative value indicating the
>>> + * error is returned.
>>> + */
>>> +SYSCALL_DEFINE3(lsm_list_modules, u64 __user *, ids, size_t __user *, size,
>>> + u32, flags)
>> I'm sorry but the size of userspace size_t is different from the kernel one
>> on 32-bit compat architectures.
> D'oh, yes, thanks for pointing that out. It would have been nice to
> have caught that before v6.8 was released, but I guess it's better
> than later.
>
>> Looks like there has to be a COMPAT_SYSCALL_DEFINE3(lsm_list_modules, ...)
>> now. Other two added lsm syscalls also have this issue.
> Considering that Linux v6.8, and by extension these syscalls, are only
> a few days old, I think I'd rather see us just modify the syscalls and
> avoid the compat baggage. I'm going to be shocked if anyone has
> shifted to using the new syscalls yet, and even if they have (!!),
> moving from a "size_t" type to a "u64" should be mostly transparent
> for the majority of native 64-bit systems. Those running the absolute
> latest kernels on 32-bit systems with custom or bleeding edge
> userspace *may* see a slight hiccup, but I think that user count is in
> the single digits, if not zero.
>
> Let's fix this quickly with /size_t/u64/ in v6.8.1 and avoid the
> compat shim if we can.
>
> Casey, do you have time to put together a patch for this (you should
> fix the call chains below the syscalls too)? If not, please let me
> know and I'll get a patch out ASAP.
Grumble. Yes, I'll get right on it.
>
> Thanks all.
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox