diff mbox series

package/openssh: backport upstream fix for 32-bit

Message ID 20220310140350.1955655-1-john@metanate.com
State Accepted
Headers show
Series package/openssh: backport upstream fix for 32-bit | expand

Commit Message

John Keeping March 10, 2022, 2:03 p.m. UTC 
sshd is broken on 32-bit systems because ppoll_time64 is used by the
application although it is not allowed by the seccomp filter.

Apply the upstream patch to fix this.

Signed-off-by: John Keeping <john@metanate.com>
---
 ...llow-ppoll_time64-in-seccomp-sandbox.patch | 31 +++++++++++++++++++
 1 file changed, 31 insertions(+)
 create mode 100644 package/openssh/0001-Allow-ppoll_time64-in-seccomp-sandbox.patch

Comments

Peter Seiderer March 10, 2022, 8:38 p.m. UTC | #1 
Hello John,

thanks for the patch, some minor nitpicks...

Better patch subject would be:

	package/openssh: add upstream patch to add seccomp ppoll_time64 support

On Thu, 10 Mar 2022 14:03:50 +0000, John Keeping <john@metanate.com> wrote:

> sshd is broken on 32-bit systems because ppoll_time64 is used by the
> application although it is not allowed by the seccomp filter.
>
> Apply the upstream patch to fix this.

Better:

  -add upstream patch ([1] to add seccomp ppoll_time64 support

[1] https://github.com/openssh/openssh-portable/commit/284b6e5394652d519e31782e3b3cdfd7b21d1a81.patch

>
> Signed-off-by: John Keeping <john@metanate.com>
> ---
>  ...llow-ppoll_time64-in-seccomp-sandbox.patch | 31 +++++++++++++++++++
>  1 file changed, 31 insertions(+)
>  create mode 100644 package/openssh/0001-Allow-ppoll_time64-in-seccomp-sandbox.patch
>
> diff --git a/package/openssh/0001-Allow-ppoll_time64-in-seccomp-sandbox.patch b/package/openssh/0001-Allow-ppoll_time64-in-seccomp-sandbox.patch
> new file mode 100644
> index 0000000000..34b309bd9a
> --- /dev/null
> +++ b/package/openssh/0001-Allow-ppoll_time64-in-seccomp-sandbox.patch
> @@ -0,0 +1,31 @@
> +From 284b6e5394652d519e31782e3b3cdfd7b21d1a81 Mon Sep 17 00:00:00 2001
> +From: Darren Tucker <dtucker@dtucker.net>
> +Date: Sat, 26 Feb 2022 14:06:14 +1100
> +Subject: [PATCH] Allow ppoll_time64 in seccomp sandbox.
> +
> +Should fix sandbox violations on (some? at least i386 and armhf) 32bit
> +Linux platforms.  Patch from chutzpahu at gentoo.org and cjwatson at
> +debian.org via bz#3396.
> +

Missing:

[Upstream: https://github.com/openssh/openssh-portable/commit/284b6e5394652d519e31782e3b3cdfd7b21d1a81.patch]
> +Signed-off-by: John Keeping <john@metanate.com>
> +---
> + sandbox-seccomp-filter.c | 3 +++
> + 1 file changed, 3 insertions(+)
> +
> +diff --git a/sandbox-seccomp-filter.c b/sandbox-seccomp-filter.c
> +index 2e065ba3..4ce80cb2 100644
> +--- a/sandbox-seccomp-filter.c
> ++++ b/sandbox-seccomp-filter.c
> +@@ -276,6 +276,9 @@ static const struct sock_filter preauth_insns[] = {
> + #ifdef __NR_ppoll
> + 	SC_ALLOW(__NR_ppoll),
> + #endif
> ++#ifdef __NR_ppoll_time64
> ++	SC_ALLOW(__NR_ppoll_time64),
> ++#endif
> + #ifdef __NR_poll
> + 	SC_ALLOW(__NR_poll),
> + #endif
> +--
> +2.35.1
> +

With this fixed you can add my

Reviewed-by: Peter Seiderer <ps.report@gmx.net>

Regards,
Peter
Arnout Vandecappelle March 10, 2022, 9:03 p.m. UTC | #2 
On 10/03/2022 21:38, Peter Seiderer wrote:
> Hello John,
> 
> thanks for the patch, some minor nitpicks...
> 
> Better patch subject would be:
> 
> 	package/openssh: add upstream patch to add seccomp ppoll_time64 support

  Applied to master with this changed.

> 
> On Thu, 10 Mar 2022 14:03:50 +0000, John Keeping <john@metanate.com> wrote:
> 
>> sshd is broken on 32-bit systems because ppoll_time64 is used by the
>> application although it is not allowed by the seccomp filter.
>>
>> Apply the upstream patch to fix this.
> 
> Better:
> 
>    -add upstream patch ([1] to add seccomp ppoll_time64 support

  Since the subject now already says that it adds seccomp ppoll_time64 support, 
this is redundant. Since I'm lazy :-), I didn't change this.

> 
> [1] https://github.com/openssh/openssh-portable/commit/284b6e5394652d519e31782e3b3cdfd7b21d1a81.patch

  There's already a reference to the upstream commit in the patch itself, so 
this is not really needed.

> 
>>
>> Signed-off-by: John Keeping <john@metanate.com>
>> ---
>>   ...llow-ppoll_time64-in-seccomp-sandbox.patch | 31 +++++++++++++++++++
>>   1 file changed, 31 insertions(+)
>>   create mode 100644 package/openssh/0001-Allow-ppoll_time64-in-seccomp-sandbox.patch
>>
>> diff --git a/package/openssh/0001-Allow-ppoll_time64-in-seccomp-sandbox.patch b/package/openssh/0001-Allow-ppoll_time64-in-seccomp-sandbox.patch
>> new file mode 100644
>> index 0000000000..34b309bd9a
>> --- /dev/null
>> +++ b/package/openssh/0001-Allow-ppoll_time64-in-seccomp-sandbox.patch
>> @@ -0,0 +1,31 @@
>> +From 284b6e5394652d519e31782e3b3cdfd7b21d1a81 Mon Sep 17 00:00:00 2001
>> +From: Darren Tucker <dtucker@dtucker.net>
>> +Date: Sat, 26 Feb 2022 14:06:14 +1100
>> +Subject: [PATCH] Allow ppoll_time64 in seccomp sandbox.
>> +
>> +Should fix sandbox violations on (some? at least i386 and armhf) 32bit
>> +Linux platforms.  Patch from chutzpahu at gentoo.org and cjwatson at
>> +debian.org via bz#3396.
>> +
> 
> Missing:
> 
> [Upstream: https://github.com/openssh/openssh-portable/commit/284b6e5394652d519e31782e3b3cdfd7b21d1a81.patch]

  Except for the signoff, the patch is literally the upstream patch, including 
the sha1 in the From line. So an upstream reference is not really needed. Still, 
it's useful so I overcame my laziness and added it.

  Regards,
  Arnout


>> +Signed-off-by: John Keeping <john@metanate.com>
>> +---
>> + sandbox-seccomp-filter.c | 3 +++
>> + 1 file changed, 3 insertions(+)
>> +
>> +diff --git a/sandbox-seccomp-filter.c b/sandbox-seccomp-filter.c
>> +index 2e065ba3..4ce80cb2 100644
>> +--- a/sandbox-seccomp-filter.c
>> ++++ b/sandbox-seccomp-filter.c
>> +@@ -276,6 +276,9 @@ static const struct sock_filter preauth_insns[] = {
>> + #ifdef __NR_ppoll
>> + 	SC_ALLOW(__NR_ppoll),
>> + #endif
>> ++#ifdef __NR_ppoll_time64
>> ++	SC_ALLOW(__NR_ppoll_time64),
>> ++#endif
>> + #ifdef __NR_poll
>> + 	SC_ALLOW(__NR_poll),
>> + #endif
>> +--
>> +2.35.1
>> +
> 
> With this fixed you can add my
> 
> Reviewed-by: Peter Seiderer <ps.report@gmx.net>
> 
> Regards,
> Peter
> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot
Peter Seiderer March 11, 2022, 7:24 a.m. UTC | #3 
Hello Arnout,

On Thu, 10 Mar 2022 22:03:23 +0100, Arnout Vandecappelle <arnout@mind.be> wrote:

> On 10/03/2022 21:38, Peter Seiderer wrote:
> > Hello John,
> >
> > thanks for the patch, some minor nitpicks...
> >
> > Better patch subject would be:
> >
> > 	package/openssh: add upstream patch to add seccomp ppoll_time64 support
>
>   Applied to master with this changed.
>
> >
> > On Thu, 10 Mar 2022 14:03:50 +0000, John Keeping <john@metanate.com> wrote:
> >
> >> sshd is broken on 32-bit systems because ppoll_time64 is used by the
> >> application although it is not allowed by the seccomp filter.
> >>
> >> Apply the upstream patch to fix this.
> >
> > Better:
> >
> >    -add upstream patch ([1] to add seccomp ppoll_time64 support
>
>   Since the subject now already says that it adds seccomp ppoll_time64 support,
> this is redundant. Since I'm lazy :-), I didn't change this.
>
> >
> > [1] https://github.com/openssh/openssh-portable/commit/284b6e5394652d519e31782e3b3cdfd7b21d1a81.patch
>
>   There's already a reference to the upstream commit in the patch itself, so
> this is not really needed.
>
> >
> >>
> >> Signed-off-by: John Keeping <john@metanate.com>
> >> ---
> >>   ...llow-ppoll_time64-in-seccomp-sandbox.patch | 31 +++++++++++++++++++
> >>   1 file changed, 31 insertions(+)
> >>   create mode 100644 package/openssh/0001-Allow-ppoll_time64-in-seccomp-sandbox.patch
> >>
> >> diff --git a/package/openssh/0001-Allow-ppoll_time64-in-seccomp-sandbox.patch b/package/openssh/0001-Allow-ppoll_time64-in-seccomp-sandbox.patch
> >> new file mode 100644
> >> index 0000000000..34b309bd9a
> >> --- /dev/null
> >> +++ b/package/openssh/0001-Allow-ppoll_time64-in-seccomp-sandbox.patch
> >> @@ -0,0 +1,31 @@
> >> +From 284b6e5394652d519e31782e3b3cdfd7b21d1a81 Mon Sep 17 00:00:00 2001
> >> +From: Darren Tucker <dtucker@dtucker.net>
> >> +Date: Sat, 26 Feb 2022 14:06:14 +1100
> >> +Subject: [PATCH] Allow ppoll_time64 in seccomp sandbox.
> >> +
> >> +Should fix sandbox violations on (some? at least i386 and armhf) 32bit
> >> +Linux platforms.  Patch from chutzpahu at gentoo.org and cjwatson at
> >> +debian.org via bz#3396.
> >> +
> >
> > Missing:
> >
> > [Upstream: https://github.com/openssh/openssh-portable/commit/284b6e5394652d519e31782e3b3cdfd7b21d1a81.patch]
>
>   Except for the signoff, the patch is literally the upstream patch, including
> the sha1 in the From line. So an upstream reference is not really needed. Still,
> it's useful so I overcame my laziness and added it.

But the sha1 alone does not tell to which git repo it belongs to, but the explicit
upstream link does (and has the nice effect to gain a one-click link to the
corresponding patch/merge-request etc.) and is a prominent remainder in case
of package version bump where the patch comes from...

Regards,
Peter

>
>   Regards,
>   Arnout
>
>
> >> +Signed-off-by: John Keeping <john@metanate.com>
> >> +---
> >> + sandbox-seccomp-filter.c | 3 +++
> >> + 1 file changed, 3 insertions(+)
> >> +
> >> +diff --git a/sandbox-seccomp-filter.c b/sandbox-seccomp-filter.c
> >> +index 2e065ba3..4ce80cb2 100644
> >> +--- a/sandbox-seccomp-filter.c
> >> ++++ b/sandbox-seccomp-filter.c
> >> +@@ -276,6 +276,9 @@ static const struct sock_filter preauth_insns[] = {
> >> + #ifdef __NR_ppoll
> >> + 	SC_ALLOW(__NR_ppoll),
> >> + #endif
> >> ++#ifdef __NR_ppoll_time64
> >> ++	SC_ALLOW(__NR_ppoll_time64),
> >> ++#endif
> >> + #ifdef __NR_poll
> >> + 	SC_ALLOW(__NR_poll),
> >> + #endif
> >> +--
> >> +2.35.1
> >> +
> >
> > With this fixed you can add my
> >
> > Reviewed-by: Peter Seiderer <ps.report@gmx.net>
> >
> > Regards,
> > Peter
> >
> > _______________________________________________
> > buildroot mailing list
> > buildroot@buildroot.org
> > https://lists.buildroot.org/mailman/listinfo/buildroot
Arnout Vandecappelle March 12, 2022, 4 p.m. UTC | #4 
On 11/03/2022 08:24, Peter Seiderer wrote:
> Hello Arnout,
> 
> On Thu, 10 Mar 2022 22:03:23 +0100, Arnout Vandecappelle <arnout@mind.be> wrote:
> 
>> On 10/03/2022 21:38, Peter Seiderer wrote:
>>> Hello John,
>>>
>>> thanks for the patch, some minor nitpicks...
>>>
>>> Better patch subject would be:
>>>
>>> 	package/openssh: add upstream patch to add seccomp ppoll_time64 support
>>
>>    Applied to master with this changed.
>>
>>>
>>> On Thu, 10 Mar 2022 14:03:50 +0000, John Keeping <john@metanate.com> wrote:
>>>
>>>> sshd is broken on 32-bit systems because ppoll_time64 is used by the
>>>> application although it is not allowed by the seccomp filter.
>>>>
>>>> Apply the upstream patch to fix this.
>>>
>>> Better:
>>>
>>>     -add upstream patch ([1] to add seccomp ppoll_time64 support
>>
>>    Since the subject now already says that it adds seccomp ppoll_time64 support,
>> this is redundant. Since I'm lazy :-), I didn't change this.
>>
>>>
>>> [1] https://github.com/openssh/openssh-portable/commit/284b6e5394652d519e31782e3b3cdfd7b21d1a81.patch
>>
>>    There's already a reference to the upstream commit in the patch itself, so
>> this is not really needed.
>>
>>>
>>>>
>>>> Signed-off-by: John Keeping <john@metanate.com>
>>>> ---
>>>>    ...llow-ppoll_time64-in-seccomp-sandbox.patch | 31 +++++++++++++++++++
>>>>    1 file changed, 31 insertions(+)
>>>>    create mode 100644 package/openssh/0001-Allow-ppoll_time64-in-seccomp-sandbox.patch
>>>>
>>>> diff --git a/package/openssh/0001-Allow-ppoll_time64-in-seccomp-sandbox.patch b/package/openssh/0001-Allow-ppoll_time64-in-seccomp-sandbox.patch
>>>> new file mode 100644
>>>> index 0000000000..34b309bd9a
>>>> --- /dev/null
>>>> +++ b/package/openssh/0001-Allow-ppoll_time64-in-seccomp-sandbox.patch
>>>> @@ -0,0 +1,31 @@
>>>> +From 284b6e5394652d519e31782e3b3cdfd7b21d1a81 Mon Sep 17 00:00:00 2001
>>>> +From: Darren Tucker <dtucker@dtucker.net>
>>>> +Date: Sat, 26 Feb 2022 14:06:14 +1100
>>>> +Subject: [PATCH] Allow ppoll_time64 in seccomp sandbox.
>>>> +
>>>> +Should fix sandbox violations on (some? at least i386 and armhf) 32bit
>>>> +Linux platforms.  Patch from chutzpahu at gentoo.org and cjwatson at
>>>> +debian.org via bz#3396.
>>>> +
>>>
>>> Missing:
>>>
>>> [Upstream: https://github.com/openssh/openssh-portable/commit/284b6e5394652d519e31782e3b3cdfd7b21d1a81.patch]
>>
>>    Except for the signoff, the patch is literally the upstream patch, including
>> the sha1 in the From line. So an upstream reference is not really needed. Still,
>> it's useful so I overcame my laziness and added it.
> 
> But the sha1 alone does not tell to which git repo it belongs to, but the explicit
> upstream link does (and has the nice effect to gain a one-click link to the
> corresponding patch/merge-request etc.) and is a prominent remainder in case
> of package version bump where the patch comes from...

  I don't know about you, but the normal way that I generate patches is to clone 
the repo and do 'git format-patch -1 <sha1>', and then add my SoB. And if it 
doesn't apply cleanly, I first do 'git cherry-pick -x -s'.

  Obviously it's helpful to have a link there. However, requiring contributors 
to go and find the upstream web frontend and construct a URL is IMHO asking a 
lot, especially since chances are that no one is ever going to actually look at 
that link. There's already plenty of steps a contributor has to go through, I 
don't want to increase the threshold even more.

  Regards,
  Arnout

> 
> Regards,
> Peter
> 
>>
>>    Regards,
>>    Arnout
>>
>>
>>>> +Signed-off-by: John Keeping <john@metanate.com>
>>>> +---
>>>> + sandbox-seccomp-filter.c | 3 +++
>>>> + 1 file changed, 3 insertions(+)
>>>> +
>>>> +diff --git a/sandbox-seccomp-filter.c b/sandbox-seccomp-filter.c
>>>> +index 2e065ba3..4ce80cb2 100644
>>>> +--- a/sandbox-seccomp-filter.c
>>>> ++++ b/sandbox-seccomp-filter.c
>>>> +@@ -276,6 +276,9 @@ static const struct sock_filter preauth_insns[] = {
>>>> + #ifdef __NR_ppoll
>>>> + 	SC_ALLOW(__NR_ppoll),
>>>> + #endif
>>>> ++#ifdef __NR_ppoll_time64
>>>> ++	SC_ALLOW(__NR_ppoll_time64),
>>>> ++#endif
>>>> + #ifdef __NR_poll
>>>> + 	SC_ALLOW(__NR_poll),
>>>> + #endif
>>>> +--
>>>> +2.35.1
>>>> +
>>>
>>> With this fixed you can add my
>>>
>>> Reviewed-by: Peter Seiderer <ps.report@gmx.net>
>>>
>>> Regards,
>>> Peter
>>>
>>> _______________________________________________
>>> buildroot mailing list
>>> buildroot@buildroot.org
>>> https://lists.buildroot.org/mailman/listinfo/buildroot
>
Yann E. MORIN March 12, 2022, 4:24 p.m. UTC | #5 
Arnout, Peter, All,

On 2022-03-12 17:00 +0100, Arnout Vandecappelle spake thusly:
> On 11/03/2022 08:24, Peter Seiderer wrote:
> >But the sha1 alone does not tell to which git repo it belongs to, but the explicit
> >upstream link does (and has the nice effect to gain a one-click link to the
> >corresponding patch/merge-request etc.) and is a prominent remainder in case
> >of package version bump where the patch comes from...
>  I don't know about you, but the normal way that I generate patches is to
> clone the repo and do 'git format-patch -1 <sha1>', and then add my SoB. And
> if it doesn't apply cleanly, I first do 'git cherry-pick -x -s'.

Exactly what I do too, with an additional tag just before my SoB:
    [yann.morin.1998@free.fr: backport from upstream]
    Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>

>  Obviously it's helpful to have a link there. However, requiring
> contributors to go and find the upstream web frontend and construct a URL is
> IMHO asking a lot, especially since chances are that no one is ever going to
> actually look at that link. There's already plenty of steps a contributor
> has to go through, I don't want to increase the threshold even more.

I fully subscribe to Arnout's position.

I only add an URL to the original patch if it was grabbed fro; a pending
PR/MR/BZ/.. or a third-party tree.

Regards,
Yann E. MORIN.
Peter Korsgaard March 18, 2022, 8:42 a.m. UTC | #6 
>>>>> "John" == John Keeping <john@metanate.com> writes:

 > sshd is broken on 32-bit systems because ppoll_time64 is used by the
 > application although it is not allowed by the seccomp filter.

 > Apply the upstream patch to fix this.

 > Signed-off-by: John Keeping <john@metanate.com>

Committed to 2022.02.x, thanks.
diff mbox series

Patch

diff --git a/package/openssh/0001-Allow-ppoll_time64-in-seccomp-sandbox.patch b/package/openssh/0001-Allow-ppoll_time64-in-seccomp-sandbox.patch
new file mode 100644
index 0000000000..34b309bd9a
--- /dev/null
+++ b/package/openssh/0001-Allow-ppoll_time64-in-seccomp-sandbox.patch
@@ -0,0 +1,31 @@ 
+From 284b6e5394652d519e31782e3b3cdfd7b21d1a81 Mon Sep 17 00:00:00 2001
+From: Darren Tucker <dtucker@dtucker.net>
+Date: Sat, 26 Feb 2022 14:06:14 +1100
+Subject: [PATCH] Allow ppoll_time64 in seccomp sandbox.
+
+Should fix sandbox violations on (some? at least i386 and armhf) 32bit
+Linux platforms.  Patch from chutzpahu at gentoo.org and cjwatson at
+debian.org via bz#3396.
+
+Signed-off-by: John Keeping <john@metanate.com>
+---
+ sandbox-seccomp-filter.c | 3 +++
+ 1 file changed, 3 insertions(+)
+
+diff --git a/sandbox-seccomp-filter.c b/sandbox-seccomp-filter.c
+index 2e065ba3..4ce80cb2 100644
+--- a/sandbox-seccomp-filter.c
++++ b/sandbox-seccomp-filter.c
+@@ -276,6 +276,9 @@ static const struct sock_filter preauth_insns[] = {
+ #ifdef __NR_ppoll
+ 	SC_ALLOW(__NR_ppoll),
+ #endif
++#ifdef __NR_ppoll_time64
++	SC_ALLOW(__NR_ppoll_time64),
++#endif
+ #ifdef __NR_poll
+ 	SC_ALLOW(__NR_poll),
+ #endif
+-- 
+2.35.1
+