Re: [PATCH v2] x86: Fix x32 System V message queue syscalls
On Sat, Dec 5, 2020 at 4:01 PM Jessica Clarke wrote: > > Ping? Can you submit patches implementing my proposal? One is your existing patch plus fixing struct msghdr, with Cc: sta...@vger.kernel.org at the bottom. The second is a removal of struct msghdr from uapi, moving it into include/inux (no uapi) if needed. The second should not cc stable. --Andy
Re: [PATCH v2] x86: Fix x32 System V message queue syscalls
On 16 Nov 2020, at 00:55, Jessica Clarke wrote:
>
> On 1 Nov 2020, at 21:01, Rich Felker wrote:
>>
>> On Sun, Nov 01, 2020 at 06:27:10PM +, Jessica Clarke wrote:
>>> On 1 Nov 2020, at 18:15, Jessica Clarke wrote:
On 1 Nov 2020, at 18:07, Andy Lutomirski wrote:
>
> On Sat, Oct 31, 2020 at 6:50 PM Rich Felker wrote:
>>
>> On Sun, Nov 01, 2020 at 01:27:35AM +, Jessica Clarke wrote:
>>> On 1 Nov 2020, at 01:22, Rich Felker wrote:
On Sat, Oct 31, 2020 at 04:30:44PM -0700, Andy Lutomirski wrote:
> cc: some libc folks
>
> On Mon, Oct 12, 2020 at 6:45 AM Jessica Clarke
> wrote:
>>
>> POSIX specifies that the first field of the supplied msgp, namely
>> mtype,
>> is a long, not a __kernel_long_t, and it's a user-defined struct due
>> to
>> the variable-length mtext field so we can't even bend the spec and
>> make
>> it a __kernel_long_t even if we wanted to. Thus we must use the
>> compat
>> syscalls on x32 to avoid buffer overreads and overflows in msgsnd and
>> msgrcv respectively.
>
> This is a mess.
>
> include/uapi/linux/msg.h has:
>
> /* message buffer for msgsnd and msgrcv calls */
> struct msgbuf {
>__kernel_long_t mtype; /* type of message */
>char mtext[1]; /* message text */
> };
>
> Your test has:
>
> struct msg_long {
> long mtype;
> char mtext[8];
> };
>
> struct msg_long_ext {
> struct msg_long msg_long;
> char mext[4];
> };
>
> and I'm unclear as to exactly what you're trying to do there with the
> "mext" part.
>
> POSIX says:
>
> The application shall ensure that the argument msgp points to a
> user-
> defined buffer that contains first a field of type long specifying
> the
> type of the message, and then a data portion that holds the data
> bytes
> of the message. The structure below is an example of what this
> user-de‐
> fined buffer might look like:
>
> struct mymsg {
> long mtype; /* Message type. */
> char mtext[1];/* Message text. */
> }
>
> NTP has this delightful piece of code:
>
> 44 typedef union {
> 45 struct msgbuf msgp;
> 46 struct {
> 47 long mtype;
> 48 int code;
> 49 struct timeval tv;
> 50 } msgb;
> 51 } MsgBuf;
>
> bluefish has:
>
> struct small_msgbuf {
> long mtype;
> char mtext[MSQ_QUEUE_SMALL_SIZE];
> } small_msgp;
>
>
> My laptop has nothing at all in /dev/mqueue.
>
> So I don't really know what the right thing to do is. Certainly if
> we're going to apply this patch, we should also fix the header. I
> almost think we should *delete* struct msgbuf from the headers, since
> it's all kinds of busted, but that will break the NTP build. Ideally
> we would go back in time and remove it from the headers.
>
> Libc people, any insight? We can probably fix the bug without
> annoying anyone given how lightly x32 is used and how lightly POSIX
> message queues are used.
If it's that outright wrong and always has been, I feel like the old
syscall numbers should just be deprecated and new ones assigned.
Otherwise, there's no way for userspace to be safe against data
corruption when run on older kernels. If there's a new syscall number,
libc can just use the new one unconditionally (giving ENOSYS on
kernels where it would be broken) or have a x32-specific
implementation that makes the old syscall and performs translation if
the new one fails with ENOSYS.
>>>
>>> That doesn't really help broken code continue to work reliably, as
>>> upgrading libc will just pull in the new syscall for a binary that's
>>> expecting the broken behaviour, unless you do symbol versioning, but
>>> then it'll just break when you next recompile the code, and there's no
>>> way for that to be diagnosed given the *application* has to define the
>>> type. But given it's application-defined I really struggle to see how
>>> any code out there is actually expecting the current x32 behaviour as
>>> you'd have to go really out of your way to find out that x32 is broken
>>> and needs __kernel_long_t. I don't think there's any way around just
>>> technically breaking ABI whilst likely really fixing ABI in
Re: [PATCH v2] x86: Fix x32 System V message queue syscalls
On 1 Nov 2020, at 21:01, Rich Felker wrote:
>
> On Sun, Nov 01, 2020 at 06:27:10PM +, Jessica Clarke wrote:
>> On 1 Nov 2020, at 18:15, Jessica Clarke wrote:
>>>
>>> On 1 Nov 2020, at 18:07, Andy Lutomirski wrote:
On Sat, Oct 31, 2020 at 6:50 PM Rich Felker wrote:
>
> On Sun, Nov 01, 2020 at 01:27:35AM +, Jessica Clarke wrote:
>> On 1 Nov 2020, at 01:22, Rich Felker wrote:
>>> On Sat, Oct 31, 2020 at 04:30:44PM -0700, Andy Lutomirski wrote:
cc: some libc folks
On Mon, Oct 12, 2020 at 6:45 AM Jessica Clarke
wrote:
>
> POSIX specifies that the first field of the supplied msgp, namely
> mtype,
> is a long, not a __kernel_long_t, and it's a user-defined struct due
> to
> the variable-length mtext field so we can't even bend the spec and
> make
> it a __kernel_long_t even if we wanted to. Thus we must use the compat
> syscalls on x32 to avoid buffer overreads and overflows in msgsnd and
> msgrcv respectively.
This is a mess.
include/uapi/linux/msg.h has:
/* message buffer for msgsnd and msgrcv calls */
struct msgbuf {
__kernel_long_t mtype; /* type of message */
char mtext[1]; /* message text */
};
Your test has:
struct msg_long {
long mtype;
char mtext[8];
};
struct msg_long_ext {
struct msg_long msg_long;
char mext[4];
};
and I'm unclear as to exactly what you're trying to do there with the
"mext" part.
POSIX says:
The application shall ensure that the argument msgp points to a
user-
defined buffer that contains first a field of type long specifying
the
type of the message, and then a data portion that holds the data
bytes
of the message. The structure below is an example of what this
user-de‐
fined buffer might look like:
struct mymsg {
long mtype; /* Message type. */
char mtext[1];/* Message text. */
}
NTP has this delightful piece of code:
44 typedef union {
45 struct msgbuf msgp;
46 struct {
47 long mtype;
48 int code;
49 struct timeval tv;
50 } msgb;
51 } MsgBuf;
bluefish has:
struct small_msgbuf {
long mtype;
char mtext[MSQ_QUEUE_SMALL_SIZE];
} small_msgp;
My laptop has nothing at all in /dev/mqueue.
So I don't really know what the right thing to do is. Certainly if
we're going to apply this patch, we should also fix the header. I
almost think we should *delete* struct msgbuf from the headers, since
it's all kinds of busted, but that will break the NTP build. Ideally
we would go back in time and remove it from the headers.
Libc people, any insight? We can probably fix the bug without
annoying anyone given how lightly x32 is used and how lightly POSIX
message queues are used.
>>>
>>> If it's that outright wrong and always has been, I feel like the old
>>> syscall numbers should just be deprecated and new ones assigned.
>>> Otherwise, there's no way for userspace to be safe against data
>>> corruption when run on older kernels. If there's a new syscall number,
>>> libc can just use the new one unconditionally (giving ENOSYS on
>>> kernels where it would be broken) or have a x32-specific
>>> implementation that makes the old syscall and performs translation if
>>> the new one fails with ENOSYS.
>>
>> That doesn't really help broken code continue to work reliably, as
>> upgrading libc will just pull in the new syscall for a binary that's
>> expecting the broken behaviour, unless you do symbol versioning, but
>> then it'll just break when you next recompile the code, and there's no
>> way for that to be diagnosed given the *application* has to define the
>> type. But given it's application-defined I really struggle to see how
>> any code out there is actually expecting the current x32 behaviour as
>> you'd have to go really out of your way to find out that x32 is broken
>> and needs __kernel_long_t. I don't think there's any way around just
>> technically breaking ABI whilst likely really fixing ABI in 99.999% of
>> cases (maybe 100%).
>
> I'm not opposed to "breaking ABI" here because the current syscall
> doesn't work unless someone wrote bogus x32-specific
Re: [PATCH v2] x86: Fix x32 System V message queue syscalls
On Sun, Nov 01, 2020 at 06:27:10PM +, Jessica Clarke wrote:
> On 1 Nov 2020, at 18:15, Jessica Clarke wrote:
> >
> > On 1 Nov 2020, at 18:07, Andy Lutomirski wrote:
> >>
> >> On Sat, Oct 31, 2020 at 6:50 PM Rich Felker wrote:
> >>>
> >>> On Sun, Nov 01, 2020 at 01:27:35AM +, Jessica Clarke wrote:
> On 1 Nov 2020, at 01:22, Rich Felker wrote:
> > On Sat, Oct 31, 2020 at 04:30:44PM -0700, Andy Lutomirski wrote:
> >> cc: some libc folks
> >>
> >> On Mon, Oct 12, 2020 at 6:45 AM Jessica Clarke
> >> wrote:
> >>>
> >>> POSIX specifies that the first field of the supplied msgp, namely
> >>> mtype,
> >>> is a long, not a __kernel_long_t, and it's a user-defined struct due
> >>> to
> >>> the variable-length mtext field so we can't even bend the spec and
> >>> make
> >>> it a __kernel_long_t even if we wanted to. Thus we must use the compat
> >>> syscalls on x32 to avoid buffer overreads and overflows in msgsnd and
> >>> msgrcv respectively.
> >>
> >> This is a mess.
> >>
> >> include/uapi/linux/msg.h has:
> >>
> >> /* message buffer for msgsnd and msgrcv calls */
> >> struct msgbuf {
> >> __kernel_long_t mtype; /* type of message */
> >> char mtext[1]; /* message text */
> >> };
> >>
> >> Your test has:
> >>
> >> struct msg_long {
> >> long mtype;
> >> char mtext[8];
> >> };
> >>
> >> struct msg_long_ext {
> >> struct msg_long msg_long;
> >> char mext[4];
> >> };
> >>
> >> and I'm unclear as to exactly what you're trying to do there with the
> >> "mext" part.
> >>
> >> POSIX says:
> >>
> >> The application shall ensure that the argument msgp points to a
> >> user-
> >> defined buffer that contains first a field of type long
> >> specifying the
> >> type of the message, and then a data portion that holds the data
> >> bytes
> >> of the message. The structure below is an example of what this
> >> user-de‐
> >> fined buffer might look like:
> >>
> >> struct mymsg {
> >> long mtype; /* Message type. */
> >> char mtext[1];/* Message text. */
> >> }
> >>
> >> NTP has this delightful piece of code:
> >>
> >> 44 typedef union {
> >> 45 struct msgbuf msgp;
> >> 46 struct {
> >> 47 long mtype;
> >> 48 int code;
> >> 49 struct timeval tv;
> >> 50 } msgb;
> >> 51 } MsgBuf;
> >>
> >> bluefish has:
> >>
> >> struct small_msgbuf {
> >> long mtype;
> >> char mtext[MSQ_QUEUE_SMALL_SIZE];
> >> } small_msgp;
> >>
> >>
> >> My laptop has nothing at all in /dev/mqueue.
> >>
> >> So I don't really know what the right thing to do is. Certainly if
> >> we're going to apply this patch, we should also fix the header. I
> >> almost think we should *delete* struct msgbuf from the headers, since
> >> it's all kinds of busted, but that will break the NTP build. Ideally
> >> we would go back in time and remove it from the headers.
> >>
> >> Libc people, any insight? We can probably fix the bug without
> >> annoying anyone given how lightly x32 is used and how lightly POSIX
> >> message queues are used.
> >
> > If it's that outright wrong and always has been, I feel like the old
> > syscall numbers should just be deprecated and new ones assigned.
> > Otherwise, there's no way for userspace to be safe against data
> > corruption when run on older kernels. If there's a new syscall number,
> > libc can just use the new one unconditionally (giving ENOSYS on
> > kernels where it would be broken) or have a x32-specific
> > implementation that makes the old syscall and performs translation if
> > the new one fails with ENOSYS.
>
> That doesn't really help broken code continue to work reliably, as
> upgrading libc will just pull in the new syscall for a binary that's
> expecting the broken behaviour, unless you do symbol versioning, but
> then it'll just break when you next recompile the code, and there's no
> way for that to be diagnosed given the *application* has to define the
> type. But given it's application-defined I really struggle to see how
> any code out there is actually expecting the current x32 behaviour as
> you'd have to go really out of your way to find out that x32 is broken
> and needs __kernel_long_t. I don't think there's any way around just
> technically breaking ABI whilst likely really fixing ABI in 99.999% of
> cases (maybe 100%).
> >>>
> >>> I'm not opposed to "breaking ABI" here because the current syscall
> >>> doesn't work unless someone wrote bogus x32-specific code to work
> >>> around it being
Re: [PATCH v2] x86: Fix x32 System V message queue syscalls
On 1 Nov 2020, at 18:15, Jessica Clarke wrote:
>
> On 1 Nov 2020, at 18:07, Andy Lutomirski wrote:
>>
>> On Sat, Oct 31, 2020 at 6:50 PM Rich Felker wrote:
>>>
>>> On Sun, Nov 01, 2020 at 01:27:35AM +, Jessica Clarke wrote:
On 1 Nov 2020, at 01:22, Rich Felker wrote:
> On Sat, Oct 31, 2020 at 04:30:44PM -0700, Andy Lutomirski wrote:
>> cc: some libc folks
>>
>> On Mon, Oct 12, 2020 at 6:45 AM Jessica Clarke wrote:
>>>
>>> POSIX specifies that the first field of the supplied msgp, namely mtype,
>>> is a long, not a __kernel_long_t, and it's a user-defined struct due to
>>> the variable-length mtext field so we can't even bend the spec and make
>>> it a __kernel_long_t even if we wanted to. Thus we must use the compat
>>> syscalls on x32 to avoid buffer overreads and overflows in msgsnd and
>>> msgrcv respectively.
>>
>> This is a mess.
>>
>> include/uapi/linux/msg.h has:
>>
>> /* message buffer for msgsnd and msgrcv calls */
>> struct msgbuf {
>> __kernel_long_t mtype; /* type of message */
>> char mtext[1]; /* message text */
>> };
>>
>> Your test has:
>>
>> struct msg_long {
>> long mtype;
>> char mtext[8];
>> };
>>
>> struct msg_long_ext {
>> struct msg_long msg_long;
>> char mext[4];
>> };
>>
>> and I'm unclear as to exactly what you're trying to do there with the
>> "mext" part.
>>
>> POSIX says:
>>
>> The application shall ensure that the argument msgp points to a
>> user-
>> defined buffer that contains first a field of type long specifying
>> the
>> type of the message, and then a data portion that holds the data
>> bytes
>> of the message. The structure below is an example of what this
>> user-de‐
>> fined buffer might look like:
>>
>> struct mymsg {
>> long mtype; /* Message type. */
>> char mtext[1];/* Message text. */
>> }
>>
>> NTP has this delightful piece of code:
>>
>> 44 typedef union {
>> 45 struct msgbuf msgp;
>> 46 struct {
>> 47 long mtype;
>> 48 int code;
>> 49 struct timeval tv;
>> 50 } msgb;
>> 51 } MsgBuf;
>>
>> bluefish has:
>>
>> struct small_msgbuf {
>> long mtype;
>> char mtext[MSQ_QUEUE_SMALL_SIZE];
>> } small_msgp;
>>
>>
>> My laptop has nothing at all in /dev/mqueue.
>>
>> So I don't really know what the right thing to do is. Certainly if
>> we're going to apply this patch, we should also fix the header. I
>> almost think we should *delete* struct msgbuf from the headers, since
>> it's all kinds of busted, but that will break the NTP build. Ideally
>> we would go back in time and remove it from the headers.
>>
>> Libc people, any insight? We can probably fix the bug without
>> annoying anyone given how lightly x32 is used and how lightly POSIX
>> message queues are used.
>
> If it's that outright wrong and always has been, I feel like the old
> syscall numbers should just be deprecated and new ones assigned.
> Otherwise, there's no way for userspace to be safe against data
> corruption when run on older kernels. If there's a new syscall number,
> libc can just use the new one unconditionally (giving ENOSYS on
> kernels where it would be broken) or have a x32-specific
> implementation that makes the old syscall and performs translation if
> the new one fails with ENOSYS.
That doesn't really help broken code continue to work reliably, as
upgrading libc will just pull in the new syscall for a binary that's
expecting the broken behaviour, unless you do symbol versioning, but
then it'll just break when you next recompile the code, and there's no
way for that to be diagnosed given the *application* has to define the
type. But given it's application-defined I really struggle to see how
any code out there is actually expecting the current x32 behaviour as
you'd have to go really out of your way to find out that x32 is broken
and needs __kernel_long_t. I don't think there's any way around just
technically breaking ABI whilst likely really fixing ABI in 99.999% of
cases (maybe 100%).
>>>
>>> I'm not opposed to "breaking ABI" here because the current syscall
>>> doesn't work unless someone wrote bogus x32-specific code to work
>>> around it being wrong. I don't particularly want to preserve any of
>>> the current behavior.
>>>
>>> What I am somewhat opposed to is making a situation where an updated
>>> libc can't be safe against getting run on a kernel with a broken
>>> version of the syscall and silently corrupting data. I'm flexible
>>> about how avoiding tha tis achieved.
>>
Re: [PATCH v2] x86: Fix x32 System V message queue syscalls
On 1 Nov 2020, at 18:07, Andy Lutomirski wrote:
>
> On Sat, Oct 31, 2020 at 6:50 PM Rich Felker wrote:
>>
>> On Sun, Nov 01, 2020 at 01:27:35AM +, Jessica Clarke wrote:
>>> On 1 Nov 2020, at 01:22, Rich Felker wrote:
On Sat, Oct 31, 2020 at 04:30:44PM -0700, Andy Lutomirski wrote:
> cc: some libc folks
>
> On Mon, Oct 12, 2020 at 6:45 AM Jessica Clarke wrote:
>>
>> POSIX specifies that the first field of the supplied msgp, namely mtype,
>> is a long, not a __kernel_long_t, and it's a user-defined struct due to
>> the variable-length mtext field so we can't even bend the spec and make
>> it a __kernel_long_t even if we wanted to. Thus we must use the compat
>> syscalls on x32 to avoid buffer overreads and overflows in msgsnd and
>> msgrcv respectively.
>
> This is a mess.
>
> include/uapi/linux/msg.h has:
>
> /* message buffer for msgsnd and msgrcv calls */
> struct msgbuf {
> __kernel_long_t mtype; /* type of message */
> char mtext[1]; /* message text */
> };
>
> Your test has:
>
> struct msg_long {
> long mtype;
> char mtext[8];
> };
>
> struct msg_long_ext {
> struct msg_long msg_long;
> char mext[4];
> };
>
> and I'm unclear as to exactly what you're trying to do there with the
> "mext" part.
>
> POSIX says:
>
> The application shall ensure that the argument msgp points to a
> user-
> defined buffer that contains first a field of type long specifying
> the
> type of the message, and then a data portion that holds the data
> bytes
> of the message. The structure below is an example of what this
> user-de‐
> fined buffer might look like:
>
> struct mymsg {
> long mtype; /* Message type. */
> char mtext[1];/* Message text. */
> }
>
> NTP has this delightful piece of code:
>
> 44 typedef union {
> 45 struct msgbuf msgp;
> 46 struct {
> 47 long mtype;
> 48 int code;
> 49 struct timeval tv;
> 50 } msgb;
> 51 } MsgBuf;
>
> bluefish has:
>
> struct small_msgbuf {
> long mtype;
> char mtext[MSQ_QUEUE_SMALL_SIZE];
> } small_msgp;
>
>
> My laptop has nothing at all in /dev/mqueue.
>
> So I don't really know what the right thing to do is. Certainly if
> we're going to apply this patch, we should also fix the header. I
> almost think we should *delete* struct msgbuf from the headers, since
> it's all kinds of busted, but that will break the NTP build. Ideally
> we would go back in time and remove it from the headers.
>
> Libc people, any insight? We can probably fix the bug without
> annoying anyone given how lightly x32 is used and how lightly POSIX
> message queues are used.
If it's that outright wrong and always has been, I feel like the old
syscall numbers should just be deprecated and new ones assigned.
Otherwise, there's no way for userspace to be safe against data
corruption when run on older kernels. If there's a new syscall number,
libc can just use the new one unconditionally (giving ENOSYS on
kernels where it would be broken) or have a x32-specific
implementation that makes the old syscall and performs translation if
the new one fails with ENOSYS.
>>>
>>> That doesn't really help broken code continue to work reliably, as
>>> upgrading libc will just pull in the new syscall for a binary that's
>>> expecting the broken behaviour, unless you do symbol versioning, but
>>> then it'll just break when you next recompile the code, and there's no
>>> way for that to be diagnosed given the *application* has to define the
>>> type. But given it's application-defined I really struggle to see how
>>> any code out there is actually expecting the current x32 behaviour as
>>> you'd have to go really out of your way to find out that x32 is broken
>>> and needs __kernel_long_t. I don't think there's any way around just
>>> technically breaking ABI whilst likely really fixing ABI in 99.999% of
>>> cases (maybe 100%).
>>
>> I'm not opposed to "breaking ABI" here because the current syscall
>> doesn't work unless someone wrote bogus x32-specific code to work
>> around it being wrong. I don't particularly want to preserve any of
>> the current behavior.
>>
>> What I am somewhat opposed to is making a situation where an updated
>> libc can't be safe against getting run on a kernel with a broken
>> version of the syscall and silently corrupting data. I'm flexible
>> about how avoiding tha tis achieved.
>
> If we're sufficiently confident that we won't regress anything by
> fixing the bug, I propose we do the following. First, we commit a fix
> that's
Re: [PATCH v2] x86: Fix x32 System V message queue syscalls
On Sat, Oct 31, 2020 at 6:50 PM Rich Felker wrote:
>
> On Sun, Nov 01, 2020 at 01:27:35AM +, Jessica Clarke wrote:
> > On 1 Nov 2020, at 01:22, Rich Felker wrote:
> > > On Sat, Oct 31, 2020 at 04:30:44PM -0700, Andy Lutomirski wrote:
> > >> cc: some libc folks
> > >>
> > >> On Mon, Oct 12, 2020 at 6:45 AM Jessica Clarke wrote:
> > >>>
> > >>> POSIX specifies that the first field of the supplied msgp, namely mtype,
> > >>> is a long, not a __kernel_long_t, and it's a user-defined struct due to
> > >>> the variable-length mtext field so we can't even bend the spec and make
> > >>> it a __kernel_long_t even if we wanted to. Thus we must use the compat
> > >>> syscalls on x32 to avoid buffer overreads and overflows in msgsnd and
> > >>> msgrcv respectively.
> > >>
> > >> This is a mess.
> > >>
> > >> include/uapi/linux/msg.h has:
> > >>
> > >> /* message buffer for msgsnd and msgrcv calls */
> > >> struct msgbuf {
> > >>__kernel_long_t mtype; /* type of message */
> > >>char mtext[1]; /* message text */
> > >> };
> > >>
> > >> Your test has:
> > >>
> > >> struct msg_long {
> > >>long mtype;
> > >>char mtext[8];
> > >> };
> > >>
> > >> struct msg_long_ext {
> > >>struct msg_long msg_long;
> > >>char mext[4];
> > >> };
> > >>
> > >> and I'm unclear as to exactly what you're trying to do there with the
> > >> "mext" part.
> > >>
> > >> POSIX says:
> > >>
> > >> The application shall ensure that the argument msgp points to a
> > >> user-
> > >> defined buffer that contains first a field of type long
> > >> specifying the
> > >> type of the message, and then a data portion that holds the data
> > >> bytes
> > >> of the message. The structure below is an example of what this
> > >> user-de‐
> > >> fined buffer might look like:
> > >>
> > >> struct mymsg {
> > >> long mtype; /* Message type. */
> > >> char mtext[1];/* Message text. */
> > >> }
> > >>
> > >> NTP has this delightful piece of code:
> > >>
> > >> 44 typedef union {
> > >> 45 struct msgbuf msgp;
> > >> 46 struct {
> > >> 47 long mtype;
> > >> 48 int code;
> > >> 49 struct timeval tv;
> > >> 50 } msgb;
> > >> 51 } MsgBuf;
> > >>
> > >> bluefish has:
> > >>
> > >> struct small_msgbuf {
> > >> long mtype;
> > >> char mtext[MSQ_QUEUE_SMALL_SIZE];
> > >> } small_msgp;
> > >>
> > >>
> > >> My laptop has nothing at all in /dev/mqueue.
> > >>
> > >> So I don't really know what the right thing to do is. Certainly if
> > >> we're going to apply this patch, we should also fix the header. I
> > >> almost think we should *delete* struct msgbuf from the headers, since
> > >> it's all kinds of busted, but that will break the NTP build. Ideally
> > >> we would go back in time and remove it from the headers.
> > >>
> > >> Libc people, any insight? We can probably fix the bug without
> > >> annoying anyone given how lightly x32 is used and how lightly POSIX
> > >> message queues are used.
> > >
> > > If it's that outright wrong and always has been, I feel like the old
> > > syscall numbers should just be deprecated and new ones assigned.
> > > Otherwise, there's no way for userspace to be safe against data
> > > corruption when run on older kernels. If there's a new syscall number,
> > > libc can just use the new one unconditionally (giving ENOSYS on
> > > kernels where it would be broken) or have a x32-specific
> > > implementation that makes the old syscall and performs translation if
> > > the new one fails with ENOSYS.
> >
> > That doesn't really help broken code continue to work reliably, as
> > upgrading libc will just pull in the new syscall for a binary that's
> > expecting the broken behaviour, unless you do symbol versioning, but
> > then it'll just break when you next recompile the code, and there's no
> > way for that to be diagnosed given the *application* has to define the
> > type. But given it's application-defined I really struggle to see how
> > any code out there is actually expecting the current x32 behaviour as
> > you'd have to go really out of your way to find out that x32 is broken
> > and needs __kernel_long_t. I don't think there's any way around just
> > technically breaking ABI whilst likely really fixing ABI in 99.999% of
> > cases (maybe 100%).
>
> I'm not opposed to "breaking ABI" here because the current syscall
> doesn't work unless someone wrote bogus x32-specific code to work
> around it being wrong. I don't particularly want to preserve any of
> the current behavior.
>
> What I am somewhat opposed to is making a situation where an updated
> libc can't be safe against getting run on a kernel with a broken
> version of the syscall and silently corrupting data. I'm flexible
> about how avoiding tha tis achieved.
If we're sufficiently confident that we won't regress anything by
fixing the bug, I propose we do the following. First, we
Re: [PATCH v2] x86: Fix x32 System V message queue syscalls
On Sun, Nov 01, 2020 at 01:27:35AM +, Jessica Clarke wrote:
> On 1 Nov 2020, at 01:22, Rich Felker wrote:
> > On Sat, Oct 31, 2020 at 04:30:44PM -0700, Andy Lutomirski wrote:
> >> cc: some libc folks
> >>
> >> On Mon, Oct 12, 2020 at 6:45 AM Jessica Clarke wrote:
> >>>
> >>> POSIX specifies that the first field of the supplied msgp, namely mtype,
> >>> is a long, not a __kernel_long_t, and it's a user-defined struct due to
> >>> the variable-length mtext field so we can't even bend the spec and make
> >>> it a __kernel_long_t even if we wanted to. Thus we must use the compat
> >>> syscalls on x32 to avoid buffer overreads and overflows in msgsnd and
> >>> msgrcv respectively.
> >>
> >> This is a mess.
> >>
> >> include/uapi/linux/msg.h has:
> >>
> >> /* message buffer for msgsnd and msgrcv calls */
> >> struct msgbuf {
> >>__kernel_long_t mtype; /* type of message */
> >>char mtext[1]; /* message text */
> >> };
> >>
> >> Your test has:
> >>
> >> struct msg_long {
> >>long mtype;
> >>char mtext[8];
> >> };
> >>
> >> struct msg_long_ext {
> >>struct msg_long msg_long;
> >>char mext[4];
> >> };
> >>
> >> and I'm unclear as to exactly what you're trying to do there with the
> >> "mext" part.
> >>
> >> POSIX says:
> >>
> >> The application shall ensure that the argument msgp points to a
> >> user-
> >> defined buffer that contains first a field of type long specifying
> >> the
> >> type of the message, and then a data portion that holds the data
> >> bytes
> >> of the message. The structure below is an example of what this
> >> user-de‐
> >> fined buffer might look like:
> >>
> >> struct mymsg {
> >> long mtype; /* Message type. */
> >> char mtext[1];/* Message text. */
> >> }
> >>
> >> NTP has this delightful piece of code:
> >>
> >> 44 typedef union {
> >> 45 struct msgbuf msgp;
> >> 46 struct {
> >> 47 long mtype;
> >> 48 int code;
> >> 49 struct timeval tv;
> >> 50 } msgb;
> >> 51 } MsgBuf;
> >>
> >> bluefish has:
> >>
> >> struct small_msgbuf {
> >> long mtype;
> >> char mtext[MSQ_QUEUE_SMALL_SIZE];
> >> } small_msgp;
> >>
> >>
> >> My laptop has nothing at all in /dev/mqueue.
> >>
> >> So I don't really know what the right thing to do is. Certainly if
> >> we're going to apply this patch, we should also fix the header. I
> >> almost think we should *delete* struct msgbuf from the headers, since
> >> it's all kinds of busted, but that will break the NTP build. Ideally
> >> we would go back in time and remove it from the headers.
> >>
> >> Libc people, any insight? We can probably fix the bug without
> >> annoying anyone given how lightly x32 is used and how lightly POSIX
> >> message queues are used.
> >
> > If it's that outright wrong and always has been, I feel like the old
> > syscall numbers should just be deprecated and new ones assigned.
> > Otherwise, there's no way for userspace to be safe against data
> > corruption when run on older kernels. If there's a new syscall number,
> > libc can just use the new one unconditionally (giving ENOSYS on
> > kernels where it would be broken) or have a x32-specific
> > implementation that makes the old syscall and performs translation if
> > the new one fails with ENOSYS.
>
> That doesn't really help broken code continue to work reliably, as
> upgrading libc will just pull in the new syscall for a binary that's
> expecting the broken behaviour, unless you do symbol versioning, but
> then it'll just break when you next recompile the code, and there's no
> way for that to be diagnosed given the *application* has to define the
> type. But given it's application-defined I really struggle to see how
> any code out there is actually expecting the current x32 behaviour as
> you'd have to go really out of your way to find out that x32 is broken
> and needs __kernel_long_t. I don't think there's any way around just
> technically breaking ABI whilst likely really fixing ABI in 99.999% of
> cases (maybe 100%).
I'm not opposed to "breaking ABI" here because the current syscall
doesn't work unless someone wrote bogus x32-specific code to work
around it being wrong. I don't particularly want to preserve any of
the current behavior.
What I am somewhat opposed to is making a situation where an updated
libc can't be safe against getting run on a kernel with a broken
version of the syscall and silently corrupting data. I'm flexible
about how avoiding tha tis achieved.
Rich
Re: [PATCH v2] x86: Fix x32 System V message queue syscalls
On 1 Nov 2020, at 01:22, Rich Felker wrote:
> On Sat, Oct 31, 2020 at 04:30:44PM -0700, Andy Lutomirski wrote:
>> cc: some libc folks
>>
>> On Mon, Oct 12, 2020 at 6:45 AM Jessica Clarke wrote:
>>>
>>> POSIX specifies that the first field of the supplied msgp, namely mtype,
>>> is a long, not a __kernel_long_t, and it's a user-defined struct due to
>>> the variable-length mtext field so we can't even bend the spec and make
>>> it a __kernel_long_t even if we wanted to. Thus we must use the compat
>>> syscalls on x32 to avoid buffer overreads and overflows in msgsnd and
>>> msgrcv respectively.
>>
>> This is a mess.
>>
>> include/uapi/linux/msg.h has:
>>
>> /* message buffer for msgsnd and msgrcv calls */
>> struct msgbuf {
>>__kernel_long_t mtype; /* type of message */
>>char mtext[1]; /* message text */
>> };
>>
>> Your test has:
>>
>> struct msg_long {
>>long mtype;
>>char mtext[8];
>> };
>>
>> struct msg_long_ext {
>>struct msg_long msg_long;
>>char mext[4];
>> };
>>
>> and I'm unclear as to exactly what you're trying to do there with the
>> "mext" part.
>>
>> POSIX says:
>>
>> The application shall ensure that the argument msgp points to a user-
>> defined buffer that contains first a field of type long specifying the
>> type of the message, and then a data portion that holds the data bytes
>> of the message. The structure below is an example of what this user-de‐
>> fined buffer might look like:
>>
>> struct mymsg {
>> long mtype; /* Message type. */
>> char mtext[1];/* Message text. */
>> }
>>
>> NTP has this delightful piece of code:
>>
>> 44 typedef union {
>> 45 struct msgbuf msgp;
>> 46 struct {
>> 47 long mtype;
>> 48 int code;
>> 49 struct timeval tv;
>> 50 } msgb;
>> 51 } MsgBuf;
>>
>> bluefish has:
>>
>> struct small_msgbuf {
>> long mtype;
>> char mtext[MSQ_QUEUE_SMALL_SIZE];
>> } small_msgp;
>>
>>
>> My laptop has nothing at all in /dev/mqueue.
>>
>> So I don't really know what the right thing to do is. Certainly if
>> we're going to apply this patch, we should also fix the header. I
>> almost think we should *delete* struct msgbuf from the headers, since
>> it's all kinds of busted, but that will break the NTP build. Ideally
>> we would go back in time and remove it from the headers.
>>
>> Libc people, any insight? We can probably fix the bug without
>> annoying anyone given how lightly x32 is used and how lightly POSIX
>> message queues are used.
>
> If it's that outright wrong and always has been, I feel like the old
> syscall numbers should just be deprecated and new ones assigned.
> Otherwise, there's no way for userspace to be safe against data
> corruption when run on older kernels. If there's a new syscall number,
> libc can just use the new one unconditionally (giving ENOSYS on
> kernels where it would be broken) or have a x32-specific
> implementation that makes the old syscall and performs translation if
> the new one fails with ENOSYS.
That doesn't really help broken code continue to work reliably, as
upgrading libc will just pull in the new syscall for a binary that's
expecting the broken behaviour, unless you do symbol versioning, but
then it'll just break when you next recompile the code, and there's no
way for that to be diagnosed given the *application* has to define the
type. But given it's application-defined I really struggle to see how
any code out there is actually expecting the current x32 behaviour as
you'd have to go really out of your way to find out that x32 is broken
and needs __kernel_long_t. I don't think there's any way around just
technically breaking ABI whilst likely really fixing ABI in 99.999% of
cases (maybe 100%).
Jess
Re: [PATCH v2] x86: Fix x32 System V message queue syscalls
On Sat, Oct 31, 2020 at 04:30:44PM -0700, Andy Lutomirski wrote:
> cc: some libc folks
>
> On Mon, Oct 12, 2020 at 6:45 AM Jessica Clarke wrote:
> >
> > POSIX specifies that the first field of the supplied msgp, namely mtype,
> > is a long, not a __kernel_long_t, and it's a user-defined struct due to
> > the variable-length mtext field so we can't even bend the spec and make
> > it a __kernel_long_t even if we wanted to. Thus we must use the compat
> > syscalls on x32 to avoid buffer overreads and overflows in msgsnd and
> > msgrcv respectively.
>
> This is a mess.
>
> include/uapi/linux/msg.h has:
>
> /* message buffer for msgsnd and msgrcv calls */
> struct msgbuf {
> __kernel_long_t mtype; /* type of message */
> char mtext[1]; /* message text */
> };
>
> Your test has:
>
> struct msg_long {
> long mtype;
> char mtext[8];
> };
>
> struct msg_long_ext {
> struct msg_long msg_long;
> char mext[4];
> };
>
> and I'm unclear as to exactly what you're trying to do there with the
> "mext" part.
>
> POSIX says:
>
>The application shall ensure that the argument msgp points to a user-
>defined buffer that contains first a field of type long specifying the
>type of the message, and then a data portion that holds the data bytes
>of the message. The structure below is an example of what this user-de‐
>fined buffer might look like:
>
>struct mymsg {
>long mtype; /* Message type. */
>char mtext[1];/* Message text. */
>}
>
> NTP has this delightful piece of code:
>
>44 typedef union {
>45 struct msgbuf msgp;
>46 struct {
>47 long mtype;
>48 int code;
>49 struct timeval tv;
>50 } msgb;
>51 } MsgBuf;
>
> bluefish has:
>
> struct small_msgbuf {
> long mtype;
> char mtext[MSQ_QUEUE_SMALL_SIZE];
> } small_msgp;
>
>
> My laptop has nothing at all in /dev/mqueue.
>
> So I don't really know what the right thing to do is. Certainly if
> we're going to apply this patch, we should also fix the header. I
> almost think we should *delete* struct msgbuf from the headers, since
> it's all kinds of busted, but that will break the NTP build. Ideally
> we would go back in time and remove it from the headers.
>
> Libc people, any insight? We can probably fix the bug without
> annoying anyone given how lightly x32 is used and how lightly POSIX
> message queues are used.
If it's that outright wrong and always has been, I feel like the old
syscall numbers should just be deprecated and new ones assigned.
Otherwise, there's no way for userspace to be safe against data
corruption when run on older kernels. If there's a new syscall number,
libc can just use the new one unconditionally (giving ENOSYS on
kernels where it would be broken) or have a x32-specific
implementation that makes the old syscall and performs translation if
the new one fails with ENOSYS.
Rich
Re: [PATCH v2] x86: Fix x32 System V message queue syscalls
On 31 Oct 2020, at 23:30, Andy Lutomirski wrote:
>
> cc: some libc folks
>
> On Mon, Oct 12, 2020 at 6:45 AM Jessica Clarke wrote:
>>
>> POSIX specifies that the first field of the supplied msgp, namely mtype,
>> is a long, not a __kernel_long_t, and it's a user-defined struct due to
>> the variable-length mtext field so we can't even bend the spec and make
>> it a __kernel_long_t even if we wanted to. Thus we must use the compat
>> syscalls on x32 to avoid buffer overreads and overflows in msgsnd and
>> msgrcv respectively.
>
> This is a mess.
>
> include/uapi/linux/msg.h has:
>
> /* message buffer for msgsnd and msgrcv calls */
> struct msgbuf {
>__kernel_long_t mtype; /* type of message */
>char mtext[1]; /* message text */
> };
Yeah that's just wrong, POSIX is very clear it's a plain long.
> Your test has:
>
> struct msg_long {
>long mtype;
>char mtext[8];
> };
>
> struct msg_long_ext {
>struct msg_long msg_long;
>char mext[4];
> };
>
> and I'm unclear as to exactly what you're trying to do there with the
> "mext" part.
The intent was to show that, despite the 8 being passed to
msgsnd/msgrcv (due to the `char mtext[8]`), an extra 4 bytes were being
copied when using long, namely mext, and that using __kernel_long_t was
giving the behaviour that should be observed for long, since the 4
bytes of mext in the destination buffer were left untouched and thus
still 0 from the memset.
> POSIX says:
>
> The application shall ensure that the argument msgp points to a user-
> defined buffer that contains first a field of type long specifying the
> type of the message, and then a data portion that holds the data bytes
> of the message. The structure below is an example of what this user-de‐
> fined buffer might look like:
>
> struct mymsg {
> long mtype; /* Message type. */
> char mtext[1];/* Message text. */
> }
>
> NTP has this delightful piece of code:
>
> 44 typedef union {
> 45 struct msgbuf msgp;
> 46 struct {
> 47 long mtype;
> 48 int code;
> 49 struct timeval tv;
> 50 } msgb;
> 51 } MsgBuf;
I was initially concerned by that, since msgbuf is a thing that FreeBSD
also defines but is completely different (used for console messages),
but it appears that adjtime.h, where that is defined, is only used in
libntp/adjtime.c and adjtimed/adjtimed.c, with the former only
including it if NEED_HPUX_ADJTIME, and the latter only being enabled by
configure.ac for HP-UX. So I do not believe that code has ever been
compiled on Linux (though that doesn't mean it's good code for HP-UX!).
> bluefish has:
>
> struct small_msgbuf {
> long mtype;
> char mtext[MSQ_QUEUE_SMALL_SIZE];
> } small_msgp;
Which is standards-conforming but unfortunately currently causes memory
corruption for the 4 bytes after it, just like any other correct user
of msgsend/msgrecv on x32.
> My laptop has nothing at all in /dev/mqueue.
>
> So I don't really know what the right thing to do is. Certainly if
> we're going to apply this patch, we should also fix the header. I
> almost think we should *delete* struct msgbuf from the headers, since
> it's all kinds of busted, but that will break the NTP build. Ideally
> we would go back in time and remove it from the headers.
I agree that it should not be in the header. Failing that though we
could change the type as it is just wrong on x32 and as you say nobody
should notice (and if anything it'll be because things *start* working).
> Libc people, any insight? We can probably fix the bug without
> annoying anyone given how lightly x32 is used and how lightly POSIX
> message queues are used.
That is my hope...
Jess
>>
>> Due to erroneously including the first 4 bytes of mtext in the mtype
>> this would previously also cause non-zero msgtyp arguments for msgrcv to
>> search for the wrong messages, and if sharing message queues between x32
>> and non-x32 (i386 or x86_64) processes this would previously cause mtext
>> to "move" and, depending on the direction and ABI combination, lose the
>> first 4 bytes.
>>
>> Signed-off-by: Jessica Clarke
>> ---
>>
>> I have verified that the test at the end of [1] now gives the correct
>> result on x32 ("PAYL" not "PAY" as I erroneously claimed it should be in
>> the above email) and that both i386 and amd64 give the same output with
>> that test as before.
>>
>> [1] <1156938f-a9a3-4ee9-b059-2294a0b9f...@jrtc27.com>
>>
>> Changes since v1:
>> * Uses the same syscall numbers for x32 as amd64 and the current x32
>> rather than (further) breaking ABI by allocating new ones from the
>> legacy x32 range
>>
>> arch/x86/entry/syscalls/syscall_64.tbl | 6 --
>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/x86/entry/syscalls/syscall_64.tbl
>> b/arch/x86/entry/syscalls/syscall_64.tbl
>> index f30d6ae9a..f462123f3 100644
>> ---
Re: [PATCH v2] x86: Fix x32 System V message queue syscalls
cc: some libc folks
On Mon, Oct 12, 2020 at 6:45 AM Jessica Clarke wrote:
>
> POSIX specifies that the first field of the supplied msgp, namely mtype,
> is a long, not a __kernel_long_t, and it's a user-defined struct due to
> the variable-length mtext field so we can't even bend the spec and make
> it a __kernel_long_t even if we wanted to. Thus we must use the compat
> syscalls on x32 to avoid buffer overreads and overflows in msgsnd and
> msgrcv respectively.
This is a mess.
include/uapi/linux/msg.h has:
/* message buffer for msgsnd and msgrcv calls */
struct msgbuf {
__kernel_long_t mtype; /* type of message */
char mtext[1]; /* message text */
};
Your test has:
struct msg_long {
long mtype;
char mtext[8];
};
struct msg_long_ext {
struct msg_long msg_long;
char mext[4];
};
and I'm unclear as to exactly what you're trying to do there with the
"mext" part.
POSIX says:
The application shall ensure that the argument msgp points to a user-
defined buffer that contains first a field of type long specifying the
type of the message, and then a data portion that holds the data bytes
of the message. The structure below is an example of what this user-de‐
fined buffer might look like:
struct mymsg {
long mtype; /* Message type. */
char mtext[1];/* Message text. */
}
NTP has this delightful piece of code:
44 typedef union {
45 struct msgbuf msgp;
46 struct {
47 long mtype;
48 int code;
49 struct timeval tv;
50 } msgb;
51 } MsgBuf;
bluefish has:
struct small_msgbuf {
long mtype;
char mtext[MSQ_QUEUE_SMALL_SIZE];
} small_msgp;
My laptop has nothing at all in /dev/mqueue.
So I don't really know what the right thing to do is. Certainly if
we're going to apply this patch, we should also fix the header. I
almost think we should *delete* struct msgbuf from the headers, since
it's all kinds of busted, but that will break the NTP build. Ideally
we would go back in time and remove it from the headers.
Libc people, any insight? We can probably fix the bug without
annoying anyone given how lightly x32 is used and how lightly POSIX
message queues are used.
--Andy
>
> Due to erroneously including the first 4 bytes of mtext in the mtype
> this would previously also cause non-zero msgtyp arguments for msgrcv to
> search for the wrong messages, and if sharing message queues between x32
> and non-x32 (i386 or x86_64) processes this would previously cause mtext
> to "move" and, depending on the direction and ABI combination, lose the
> first 4 bytes.
>
> Signed-off-by: Jessica Clarke
> ---
>
> I have verified that the test at the end of [1] now gives the correct
> result on x32 ("PAYL" not "PAY" as I erroneously claimed it should be in
> the above email) and that both i386 and amd64 give the same output with
> that test as before.
>
> [1] <1156938f-a9a3-4ee9-b059-2294a0b9f...@jrtc27.com>
>
> Changes since v1:
> * Uses the same syscall numbers for x32 as amd64 and the current x32
>rather than (further) breaking ABI by allocating new ones from the
>legacy x32 range
>
> arch/x86/entry/syscalls/syscall_64.tbl | 6 --
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/entry/syscalls/syscall_64.tbl
> b/arch/x86/entry/syscalls/syscall_64.tbl
> index f30d6ae9a..f462123f3 100644
> --- a/arch/x86/entry/syscalls/syscall_64.tbl
> +++ b/arch/x86/entry/syscalls/syscall_64.tbl
> @@ -77,8 +77,10 @@
> 66 common semctl sys_semctl
> 67 common shmdt sys_shmdt
> 68 common msgget sys_msgget
> -69 common msgsnd sys_msgsnd
> -70 common msgrcv sys_msgrcv
> +69 64 msgsnd sys_msgsnd
> +69 x32 msgsnd compat_sys_msgsnd
> +70 64 msgrcv sys_msgrcv
> +70 x32 msgrcv compat_sys_msgrcv
> 71 common msgctl sys_msgctl
> 72 common fcntl sys_fcntl
> 73 common flock sys_flock
> --
> 2.28.0
>
Re: [PATCH v2] x86: Fix x32 System V message queue syscalls
> On 12 Oct 2020, at 14:44, Jessica Clarke wrote:
>
> POSIX specifies that the first field of the supplied msgp, namely mtype,
> is a long, not a __kernel_long_t, and it's a user-defined struct due to
> the variable-length mtext field so we can't even bend the spec and make
> it a __kernel_long_t even if we wanted to. Thus we must use the compat
> syscalls on x32 to avoid buffer overreads and overflows in msgsnd and
> msgrcv respectively.
>
> Due to erroneously including the first 4 bytes of mtext in the mtype
> this would previously also cause non-zero msgtyp arguments for msgrcv to
> search for the wrong messages, and if sharing message queues between x32
> and non-x32 (i386 or x86_64) processes this would previously cause mtext
> to "move" and, depending on the direction and ABI combination, lose the
> first 4 bytes.
>
> Signed-off-by: Jessica Clarke
> ---
Ping?
Jess
>
> I have verified that the test at the end of [1] now gives the correct
> result on x32 ("PAYL" not "PAY" as I erroneously claimed it should be in
> the above email) and that both i386 and amd64 give the same output with
> that test as before.
>
> [1] <1156938f-a9a3-4ee9-b059-2294a0b9f...@jrtc27.com>
>
> Changes since v1:
> * Uses the same syscall numbers for x32 as amd64 and the current x32
> rather than (further) breaking ABI by allocating new ones from the
> legacy x32 range
>
> arch/x86/entry/syscalls/syscall_64.tbl | 6 --
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/entry/syscalls/syscall_64.tbl
> b/arch/x86/entry/syscalls/syscall_64.tbl
> index f30d6ae9a..f462123f3 100644
> --- a/arch/x86/entry/syscalls/syscall_64.tbl
> +++ b/arch/x86/entry/syscalls/syscall_64.tbl
> @@ -77,8 +77,10 @@
> 66common semctl sys_semctl
> 67common shmdt sys_shmdt
> 68common msgget sys_msgget
> -69 common msgsnd sys_msgsnd
> -70 common msgrcv sys_msgrcv
> +69 64 msgsnd sys_msgsnd
> +69 x32 msgsnd compat_sys_msgsnd
> +70 64 msgrcv sys_msgrcv
> +70 x32 msgrcv compat_sys_msgrcv
> 71common msgctl sys_msgctl
> 72common fcntl sys_fcntl
> 73common flock sys_flock
> --
> 2.28.0
>
