linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] riscv: Bump COMMAND_LINE_SIZE value to 1024
@ 2021-03-16 19:34 Alexandre Ghiti
  2021-03-30  5:07 ` Palmer Dabbelt
  0 siblings, 1 reply; 10+ messages in thread
From: Alexandre Ghiti @ 2021-03-16 19:34 UTC (permalink / raw)
  To: Paul Walmsley, Palmer Dabbelt, Dmitry Vyukov, linux-api,
	linux-riscv, linux-kernel
  Cc: Alexandre Ghiti

Increase COMMAND_LINE_SIZE as the current default value is too low
for syzbot kernel command line.

Reported-by: Dmitry Vyukov <dvyukov@google.com>
Signed-off-by: Alexandre Ghiti <alex@ghiti.fr>
---
 arch/riscv/include/uapi/asm/setup.h | 8 ++++++++
 1 file changed, 8 insertions(+)
 create mode 100644 arch/riscv/include/uapi/asm/setup.h

diff --git a/arch/riscv/include/uapi/asm/setup.h b/arch/riscv/include/uapi/asm/setup.h
new file mode 100644
index 000000000000..66b13a522880
--- /dev/null
+++ b/arch/riscv/include/uapi/asm/setup.h
@@ -0,0 +1,8 @@
+/* SPDX-License-Identifier: GPL-2.0-only WITH Linux-syscall-note */
+
+#ifndef _UAPI_ASM_RISCV_SETUP_H
+#define _UAPI_ASM_RISCV_SETUP_H
+
+#define COMMAND_LINE_SIZE	1024
+
+#endif /* _UAPI_ASM_RISCV_SETUP_H */
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] riscv: Bump COMMAND_LINE_SIZE value to 1024
  2021-03-16 19:34 [PATCH] riscv: Bump COMMAND_LINE_SIZE value to 1024 Alexandre Ghiti
@ 2021-03-30  5:07 ` Palmer Dabbelt
  2021-03-30 20:31   ` Maciej W. Rozycki
  0 siblings, 1 reply; 10+ messages in thread
From: Palmer Dabbelt @ 2021-03-30  5:07 UTC (permalink / raw)
  To: alex; +Cc: Paul Walmsley, dvyukov, linux-api, linux-riscv, linux-kernel, alex

On Tue, 16 Mar 2021 12:34:20 PDT (-0700), alex@ghiti.fr wrote:
> Increase COMMAND_LINE_SIZE as the current default value is too low
> for syzbot kernel command line.
>
> Reported-by: Dmitry Vyukov <dvyukov@google.com>
> Signed-off-by: Alexandre Ghiti <alex@ghiti.fr>
> ---
>  arch/riscv/include/uapi/asm/setup.h | 8 ++++++++
>  1 file changed, 8 insertions(+)
>  create mode 100644 arch/riscv/include/uapi/asm/setup.h
>
> diff --git a/arch/riscv/include/uapi/asm/setup.h b/arch/riscv/include/uapi/asm/setup.h
> new file mode 100644
> index 000000000000..66b13a522880
> --- /dev/null
> +++ b/arch/riscv/include/uapi/asm/setup.h
> @@ -0,0 +1,8 @@
> +/* SPDX-License-Identifier: GPL-2.0-only WITH Linux-syscall-note */
> +
> +#ifndef _UAPI_ASM_RISCV_SETUP_H
> +#define _UAPI_ASM_RISCV_SETUP_H
> +
> +#define COMMAND_LINE_SIZE	1024
> +
> +#endif /* _UAPI_ASM_RISCV_SETUP_H */

I put this on fixes, but it seemes like this should really be a Kconfig 
enttry.  Either way, ours was quite a bit smaller than most 
architectures and it's great that syzbot has started to find bugs, so 
I'd rather get this in sooner.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] riscv: Bump COMMAND_LINE_SIZE value to 1024
  2021-03-30  5:07 ` Palmer Dabbelt
@ 2021-03-30 20:31   ` Maciej W. Rozycki
  2021-04-02  4:37     ` Palmer Dabbelt
  0 siblings, 1 reply; 10+ messages in thread
From: Maciej W. Rozycki @ 2021-03-30 20:31 UTC (permalink / raw)
  To: Palmer Dabbelt
  Cc: alex, Paul Walmsley, dvyukov, linux-api, linux-riscv, linux-kernel

On Mon, 29 Mar 2021, Palmer Dabbelt wrote:

> > --- /dev/null
> > +++ b/arch/riscv/include/uapi/asm/setup.h
> > @@ -0,0 +1,8 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only WITH Linux-syscall-note */
> > +
> > +#ifndef _UAPI_ASM_RISCV_SETUP_H
> > +#define _UAPI_ASM_RISCV_SETUP_H
> > +
> > +#define COMMAND_LINE_SIZE	1024
> > +
> > +#endif /* _UAPI_ASM_RISCV_SETUP_H */
> 
> I put this on fixes, but it seemes like this should really be a Kconfig
> enttry.  Either way, ours was quite a bit smaller than most architectures and
> it's great that syzbot has started to find bugs, so I'd rather get this in
> sooner.

 This macro is exported as a part of the user API so it must not depend on 
Kconfig.  Also changing it (rather than say adding COMMAND_LINE_SIZE_V2 or 
switching to an entirely new data object that has its dimension set in a 
different way) requires careful evaluation as external binaries have and 
will have the value it expands to compiled in, so it's a part of the ABI 
too.

  Maciej

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] riscv: Bump COMMAND_LINE_SIZE value to 1024
  2021-03-30 20:31   ` Maciej W. Rozycki
@ 2021-04-02  4:37     ` Palmer Dabbelt
  2021-04-02  8:40       ` Dmitry Vyukov
  0 siblings, 1 reply; 10+ messages in thread
From: Palmer Dabbelt @ 2021-04-02  4:37 UTC (permalink / raw)
  To: macro; +Cc: alex, Paul Walmsley, dvyukov, linux-api, linux-riscv, linux-kernel

On Tue, 30 Mar 2021 13:31:45 PDT (-0700), macro@orcam.me.uk wrote:
> On Mon, 29 Mar 2021, Palmer Dabbelt wrote:
>
>> > --- /dev/null
>> > +++ b/arch/riscv/include/uapi/asm/setup.h
>> > @@ -0,0 +1,8 @@
>> > +/* SPDX-License-Identifier: GPL-2.0-only WITH Linux-syscall-note */
>> > +
>> > +#ifndef _UAPI_ASM_RISCV_SETUP_H
>> > +#define _UAPI_ASM_RISCV_SETUP_H
>> > +
>> > +#define COMMAND_LINE_SIZE	1024
>> > +
>> > +#endif /* _UAPI_ASM_RISCV_SETUP_H */
>>
>> I put this on fixes, but it seemes like this should really be a Kconfig
>> enttry.  Either way, ours was quite a bit smaller than most architectures and
>> it's great that syzbot has started to find bugs, so I'd rather get this in
>> sooner.
>
>  This macro is exported as a part of the user API so it must not depend on
> Kconfig.  Also changing it (rather than say adding COMMAND_LINE_SIZE_V2 or
> switching to an entirely new data object that has its dimension set in a
> different way) requires careful evaluation as external binaries have and
> will have the value it expands to compiled in, so it's a part of the ABI
> too.

Thanks, I didn't realize this was part of the user BI.  In that case we 
really can't chage it, so we'll have to sort out some other way do fix 
whatever is going on.

I've dropped this from fixes.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] riscv: Bump COMMAND_LINE_SIZE value to 1024
  2021-04-02  4:37     ` Palmer Dabbelt
@ 2021-04-02  8:40       ` Dmitry Vyukov
  2021-04-02  8:58         ` David Abdurachmanov
  0 siblings, 1 reply; 10+ messages in thread
From: Dmitry Vyukov @ 2021-04-02  8:40 UTC (permalink / raw)
  To: Palmer Dabbelt
  Cc: macro, Alex Ghiti, Paul Walmsley, Linux API, linux-riscv, LKML

On Fri, Apr 2, 2021 at 6:37 AM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>
> On Tue, 30 Mar 2021 13:31:45 PDT (-0700), macro@orcam.me.uk wrote:
> > On Mon, 29 Mar 2021, Palmer Dabbelt wrote:
> >
> >> > --- /dev/null
> >> > +++ b/arch/riscv/include/uapi/asm/setup.h
> >> > @@ -0,0 +1,8 @@
> >> > +/* SPDX-License-Identifier: GPL-2.0-only WITH Linux-syscall-note */
> >> > +
> >> > +#ifndef _UAPI_ASM_RISCV_SETUP_H
> >> > +#define _UAPI_ASM_RISCV_SETUP_H
> >> > +
> >> > +#define COMMAND_LINE_SIZE 1024
> >> > +
> >> > +#endif /* _UAPI_ASM_RISCV_SETUP_H */
> >>
> >> I put this on fixes, but it seemes like this should really be a Kconfig
> >> enttry.  Either way, ours was quite a bit smaller than most architectures and
> >> it's great that syzbot has started to find bugs, so I'd rather get this in
> >> sooner.
> >
> >  This macro is exported as a part of the user API so it must not depend on
> > Kconfig.  Also changing it (rather than say adding COMMAND_LINE_SIZE_V2 or
> > switching to an entirely new data object that has its dimension set in a
> > different way) requires careful evaluation as external binaries have and
> > will have the value it expands to compiled in, so it's a part of the ABI
> > too.
>
> Thanks, I didn't realize this was part of the user BI.  In that case we
> really can't chage it, so we'll have to sort out some other way do fix
> whatever is going on.
>
> I've dropped this from fixes.

Does increasing COMMAND_LINE_SIZE break user-space binaries? I would
expect it to work the same way as adding new enum values, or adding
fields at the end of versioned structs, etc.
I would assume the old bootloaders/etc will only support up to the
old, smaller max command line size, while the kernel will support
larger command line size, which is fine.
However, if something copies /proc/cmdline into a fixed-size buffer
and expects that to work, that will break... that's quite unfortunate
user-space code... is it what we afraid of?

Alternatively, could expose the same COMMAND_LINE_SIZE, but internally
support a larger command line?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] riscv: Bump COMMAND_LINE_SIZE value to 1024
  2021-04-02  8:40       ` Dmitry Vyukov
@ 2021-04-02  8:58         ` David Abdurachmanov
  2021-04-02 18:33           ` Maciej W. Rozycki
  0 siblings, 1 reply; 10+ messages in thread
From: David Abdurachmanov @ 2021-04-02  8:58 UTC (permalink / raw)
  To: Dmitry Vyukov
  Cc: Palmer Dabbelt, macro, Alex Ghiti, Paul Walmsley, Linux API,
	linux-riscv, LKML

On Fri, Apr 2, 2021 at 11:43 AM Dmitry Vyukov <dvyukov@google.com> wrote:
>
> On Fri, Apr 2, 2021 at 6:37 AM Palmer Dabbelt <palmer@dabbelt.com> wrote:
> >
> > On Tue, 30 Mar 2021 13:31:45 PDT (-0700), macro@orcam.me.uk wrote:
> > > On Mon, 29 Mar 2021, Palmer Dabbelt wrote:
> > >
> > >> > --- /dev/null
> > >> > +++ b/arch/riscv/include/uapi/asm/setup.h
> > >> > @@ -0,0 +1,8 @@
> > >> > +/* SPDX-License-Identifier: GPL-2.0-only WITH Linux-syscall-note */
> > >> > +
> > >> > +#ifndef _UAPI_ASM_RISCV_SETUP_H
> > >> > +#define _UAPI_ASM_RISCV_SETUP_H
> > >> > +
> > >> > +#define COMMAND_LINE_SIZE 1024
> > >> > +
> > >> > +#endif /* _UAPI_ASM_RISCV_SETUP_H */
> > >>
> > >> I put this on fixes, but it seemes like this should really be a Kconfig
> > >> enttry.  Either way, ours was quite a bit smaller than most architectures and
> > >> it's great that syzbot has started to find bugs, so I'd rather get this in
> > >> sooner.
> > >
> > >  This macro is exported as a part of the user API so it must not depend on
> > > Kconfig.  Also changing it (rather than say adding COMMAND_LINE_SIZE_V2 or
> > > switching to an entirely new data object that has its dimension set in a
> > > different way) requires careful evaluation as external binaries have and
> > > will have the value it expands to compiled in, so it's a part of the ABI
> > > too.
> >
> > Thanks, I didn't realize this was part of the user BI.  In that case we
> > really can't chage it, so we'll have to sort out some other way do fix
> > whatever is going on.
> >
> > I've dropped this from fixes.
>
> Does increasing COMMAND_LINE_SIZE break user-space binaries? I would
> expect it to work the same way as adding new enum values, or adding
> fields at the end of versioned structs, etc.
> I would assume the old bootloaders/etc will only support up to the
> old, smaller max command line size, while the kernel will support
> larger command line size, which is fine.
> However, if something copies /proc/cmdline into a fixed-size buffer
> and expects that to work, that will break... that's quite unfortunate
> user-space code... is it what we afraid of?
>
> Alternatively, could expose the same COMMAND_LINE_SIZE, but internally
> support a larger command line?

Looking at kernel commit history I see PowerPC switched from 512 to
2048, and I don't see complaints about the ABI on the mailing list.

If COMMAND_LINE_SIZE is used by user space applications and we
increase it there shouldn't be problems. I would expect things to
work, but just get truncated boot args? That is the application will
continue only to look at the initial 512 chars.

https://linuxppc-dev.ozlabs.narkive.com/m4cj8nBa/patch-1-1-powerpc-increase-command-line-size-to-2048-from-512

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] riscv: Bump COMMAND_LINE_SIZE value to 1024
  2021-04-02  8:58         ` David Abdurachmanov
@ 2021-04-02 18:33           ` Maciej W. Rozycki
  2021-04-23  2:57             ` Palmer Dabbelt
  0 siblings, 1 reply; 10+ messages in thread
From: Maciej W. Rozycki @ 2021-04-02 18:33 UTC (permalink / raw)
  To: David Abdurachmanov
  Cc: Dmitry Vyukov, Palmer Dabbelt, Alex Ghiti, Paul Walmsley,
	Linux API, linux-riscv, LKML

On Fri, 2 Apr 2021, David Abdurachmanov wrote:

> > > >  This macro is exported as a part of the user API so it must not depend on
> > > > Kconfig.  Also changing it (rather than say adding COMMAND_LINE_SIZE_V2 or
> > > > switching to an entirely new data object that has its dimension set in a
> > > > different way) requires careful evaluation as external binaries have and
> > > > will have the value it expands to compiled in, so it's a part of the ABI
> > > > too.
> > >
> > > Thanks, I didn't realize this was part of the user BI.  In that case we
> > > really can't chage it, so we'll have to sort out some other way do fix
> > > whatever is going on.
> > >
> > > I've dropped this from fixes.
> >
> > Does increasing COMMAND_LINE_SIZE break user-space binaries? I would
> > expect it to work the same way as adding new enum values, or adding
> > fields at the end of versioned structs, etc.
> > I would assume the old bootloaders/etc will only support up to the
> > old, smaller max command line size, while the kernel will support
> > larger command line size, which is fine.
> > However, if something copies /proc/cmdline into a fixed-size buffer
> > and expects that to work, that will break... that's quite unfortunate
> > user-space code... is it what we afraid of?
> >
> > Alternatively, could expose the same COMMAND_LINE_SIZE, but internally
> > support a larger command line?
> 
> Looking at kernel commit history I see PowerPC switched from 512 to
> 2048, and I don't see complaints about the ABI on the mailing list.
> 
> If COMMAND_LINE_SIZE is used by user space applications and we
> increase it there shouldn't be problems. I would expect things to
> work, but just get truncated boot args? That is the application will
> continue only to look at the initial 512 chars.

 The macro is in an include/uapi header, so it's exported to the userland 
and a part of the user API.  I don't know what the consequences are for 
the RISC-V port specifically, but it has raised my attention, and I think 
it has to be investigated.

 Perhaps it's OK to change it after all, but you'd have to go through 
known/potential users of this macro.  I guess there shouldn't be that many 
of them.

 In any case it cannot depend on Kconfig, because the userland won't have 
access to the configuration, and then presumably wants to handle any and 
all.

  Maciej

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] riscv: Bump COMMAND_LINE_SIZE value to 1024
  2021-04-02 18:33           ` Maciej W. Rozycki
@ 2021-04-23  2:57             ` Palmer Dabbelt
  2021-06-21  7:11               ` Alex Ghiti
  0 siblings, 1 reply; 10+ messages in thread
From: Palmer Dabbelt @ 2021-04-23  2:57 UTC (permalink / raw)
  To: macro
  Cc: david.abdurachmanov, dvyukov, alex, Paul Walmsley, linux-api,
	linux-riscv, linux-kernel

On Fri, 02 Apr 2021 11:33:30 PDT (-0700), macro@orcam.me.uk wrote:
> On Fri, 2 Apr 2021, David Abdurachmanov wrote:
>
>> > > >  This macro is exported as a part of the user API so it must not depend on
>> > > > Kconfig.  Also changing it (rather than say adding COMMAND_LINE_SIZE_V2 or
>> > > > switching to an entirely new data object that has its dimension set in a
>> > > > different way) requires careful evaluation as external binaries have and
>> > > > will have the value it expands to compiled in, so it's a part of the ABI
>> > > > too.
>> > >
>> > > Thanks, I didn't realize this was part of the user BI.  In that case we
>> > > really can't chage it, so we'll have to sort out some other way do fix
>> > > whatever is going on.
>> > >
>> > > I've dropped this from fixes.
>> >
>> > Does increasing COMMAND_LINE_SIZE break user-space binaries? I would
>> > expect it to work the same way as adding new enum values, or adding
>> > fields at the end of versioned structs, etc.
>> > I would assume the old bootloaders/etc will only support up to the
>> > old, smaller max command line size, while the kernel will support
>> > larger command line size, which is fine.
>> > However, if something copies /proc/cmdline into a fixed-size buffer
>> > and expects that to work, that will break... that's quite unfortunate
>> > user-space code... is it what we afraid of?
>> >
>> > Alternatively, could expose the same COMMAND_LINE_SIZE, but internally
>> > support a larger command line?
>>
>> Looking at kernel commit history I see PowerPC switched from 512 to
>> 2048, and I don't see complaints about the ABI on the mailing list.
>>
>> If COMMAND_LINE_SIZE is used by user space applications and we
>> increase it there shouldn't be problems. I would expect things to
>> work, but just get truncated boot args? That is the application will
>> continue only to look at the initial 512 chars.
>
>  The macro is in an include/uapi header, so it's exported to the userland
> and a part of the user API.  I don't know what the consequences are for
> the RISC-V port specifically, but it has raised my attention, and I think
> it has to be investigated.
>
>  Perhaps it's OK to change it after all, but you'd have to go through
> known/potential users of this macro.  I guess there shouldn't be that many
> of them.
>
>  In any case it cannot depend on Kconfig, because the userland won't have
> access to the configuration, and then presumably wants to handle any and
> all.

It kind of feels to me like COMMAND_LINE_SIZE shouldn't have been part 
of the UABI to begin with.  I sent a patch to remove it from the 
asm-generic UABI, let's see if anyone knows of a reason it should be 
UABI:

https://lore.kernel.org/linux-arch/20210423025545.313965-1-palmer@dabbelt.com/T/#u

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] riscv: Bump COMMAND_LINE_SIZE value to 1024
  2021-04-23  2:57             ` Palmer Dabbelt
@ 2021-06-21  7:11               ` Alex Ghiti
  2022-11-10 21:01                 ` Dmitry Vyukov
  0 siblings, 1 reply; 10+ messages in thread
From: Alex Ghiti @ 2021-06-21  7:11 UTC (permalink / raw)
  To: Palmer Dabbelt, macro
  Cc: david.abdurachmanov, dvyukov, Paul Walmsley, linux-api,
	linux-riscv, linux-kernel

Hi Palmer,

Le 23/04/2021 à 04:57, Palmer Dabbelt a écrit :
> On Fri, 02 Apr 2021 11:33:30 PDT (-0700), macro@orcam.me.uk wrote:
>> On Fri, 2 Apr 2021, David Abdurachmanov wrote:
>>
>>> > > >  This macro is exported as a part of the user API so it must 
>>> not depend on
>>> > > > Kconfig.  Also changing it (rather than say adding 
>>> COMMAND_LINE_SIZE_V2 or
>>> > > > switching to an entirely new data object that has its dimension 
>>> set in a
>>> > > > different way) requires careful evaluation as external binaries 
>>> have and
>>> > > > will have the value it expands to compiled in, so it's a part 
>>> of the ABI
>>> > > > too.
>>> > >
>>> > > Thanks, I didn't realize this was part of the user BI.  In that 
>>> case we
>>> > > really can't chage it, so we'll have to sort out some other way 
>>> do fix
>>> > > whatever is going on.
>>> > >
>>> > > I've dropped this from fixes.
>>> >
>>> > Does increasing COMMAND_LINE_SIZE break user-space binaries? I would
>>> > expect it to work the same way as adding new enum values, or adding
>>> > fields at the end of versioned structs, etc.
>>> > I would assume the old bootloaders/etc will only support up to the
>>> > old, smaller max command line size, while the kernel will support
>>> > larger command line size, which is fine.
>>> > However, if something copies /proc/cmdline into a fixed-size buffer
>>> > and expects that to work, that will break... that's quite unfortunate
>>> > user-space code... is it what we afraid of?
>>> >
>>> > Alternatively, could expose the same COMMAND_LINE_SIZE, but internally
>>> > support a larger command line?
>>>
>>> Looking at kernel commit history I see PowerPC switched from 512 to
>>> 2048, and I don't see complaints about the ABI on the mailing list.
>>>
>>> If COMMAND_LINE_SIZE is used by user space applications and we
>>> increase it there shouldn't be problems. I would expect things to
>>> work, but just get truncated boot args? That is the application will
>>> continue only to look at the initial 512 chars.
>>
>>  The macro is in an include/uapi header, so it's exported to the userland
>> and a part of the user API.  I don't know what the consequences are for
>> the RISC-V port specifically, but it has raised my attention, and I think
>> it has to be investigated.
>>
>>  Perhaps it's OK to change it after all, but you'd have to go through
>> known/potential users of this macro.  I guess there shouldn't be that 
>> many
>> of them.
>>
>>  In any case it cannot depend on Kconfig, because the userland won't have
>> access to the configuration, and then presumably wants to handle any and
>> all.
> 
> It kind of feels to me like COMMAND_LINE_SIZE shouldn't have been part 
> of the UABI to begin with.  I sent a patch to remove it from the 
> asm-generic UABI, let's see if anyone knows of a reason it should be UABI:
> 
> https://lore.kernel.org/linux-arch/20210423025545.313965-1-palmer@dabbelt.com/T/#u 

Arnd seemed to agree with you about removing COMMAND_LINE_SIZE from the 
UABI, any progress on your side?

Thanks,

Alex

> 
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] riscv: Bump COMMAND_LINE_SIZE value to 1024
  2021-06-21  7:11               ` Alex Ghiti
@ 2022-11-10 21:01                 ` Dmitry Vyukov
  0 siblings, 0 replies; 10+ messages in thread
From: Dmitry Vyukov @ 2022-11-10 21:01 UTC (permalink / raw)
  To: Alex Ghiti
  Cc: Palmer Dabbelt, macro, david.abdurachmanov, Paul Walmsley,
	linux-api, linux-riscv, linux-kernel

On Mon, 21 Jun 2021 at 00:11, Alex Ghiti <alex@ghiti.fr> wrote:
>
> Hi Palmer,
>
> Le 23/04/2021 à 04:57, Palmer Dabbelt a écrit :
> > On Fri, 02 Apr 2021 11:33:30 PDT (-0700), macro@orcam.me.uk wrote:
> >> On Fri, 2 Apr 2021, David Abdurachmanov wrote:
> >>
> >>> > > >  This macro is exported as a part of the user API so it must
> >>> not depend on
> >>> > > > Kconfig.  Also changing it (rather than say adding
> >>> COMMAND_LINE_SIZE_V2 or
> >>> > > > switching to an entirely new data object that has its dimension
> >>> set in a
> >>> > > > different way) requires careful evaluation as external binaries
> >>> have and
> >>> > > > will have the value it expands to compiled in, so it's a part
> >>> of the ABI
> >>> > > > too.
> >>> > >
> >>> > > Thanks, I didn't realize this was part of the user BI.  In that
> >>> case we
> >>> > > really can't chage it, so we'll have to sort out some other way
> >>> do fix
> >>> > > whatever is going on.
> >>> > >
> >>> > > I've dropped this from fixes.
> >>> >
> >>> > Does increasing COMMAND_LINE_SIZE break user-space binaries? I would
> >>> > expect it to work the same way as adding new enum values, or adding
> >>> > fields at the end of versioned structs, etc.
> >>> > I would assume the old bootloaders/etc will only support up to the
> >>> > old, smaller max command line size, while the kernel will support
> >>> > larger command line size, which is fine.
> >>> > However, if something copies /proc/cmdline into a fixed-size buffer
> >>> > and expects that to work, that will break... that's quite unfortunate
> >>> > user-space code... is it what we afraid of?
> >>> >
> >>> > Alternatively, could expose the same COMMAND_LINE_SIZE, but internally
> >>> > support a larger command line?
> >>>
> >>> Looking at kernel commit history I see PowerPC switched from 512 to
> >>> 2048, and I don't see complaints about the ABI on the mailing list.
> >>>
> >>> If COMMAND_LINE_SIZE is used by user space applications and we
> >>> increase it there shouldn't be problems. I would expect things to
> >>> work, but just get truncated boot args? That is the application will
> >>> continue only to look at the initial 512 chars.
> >>
> >>  The macro is in an include/uapi header, so it's exported to the userland
> >> and a part of the user API.  I don't know what the consequences are for
> >> the RISC-V port specifically, but it has raised my attention, and I think
> >> it has to be investigated.
> >>
> >>  Perhaps it's OK to change it after all, but you'd have to go through
> >> known/potential users of this macro.  I guess there shouldn't be that
> >> many
> >> of them.
> >>
> >>  In any case it cannot depend on Kconfig, because the userland won't have
> >> access to the configuration, and then presumably wants to handle any and
> >> all.
> >
> > It kind of feels to me like COMMAND_LINE_SIZE shouldn't have been part
> > of the UABI to begin with.  I sent a patch to remove it from the
> > asm-generic UABI, let's see if anyone knows of a reason it should be UABI:
> >
> > https://lore.kernel.org/linux-arch/20210423025545.313965-1-palmer@dabbelt.com/T/#u
>
> Arnd seemed to agree with you about removing COMMAND_LINE_SIZE from the
> UABI, any progress on your side?

Was this ever merged? Don't see this even in linux-next.

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2022-11-10 21:01 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-16 19:34 [PATCH] riscv: Bump COMMAND_LINE_SIZE value to 1024 Alexandre Ghiti
2021-03-30  5:07 ` Palmer Dabbelt
2021-03-30 20:31   ` Maciej W. Rozycki
2021-04-02  4:37     ` Palmer Dabbelt
2021-04-02  8:40       ` Dmitry Vyukov
2021-04-02  8:58         ` David Abdurachmanov
2021-04-02 18:33           ` Maciej W. Rozycki
2021-04-23  2:57             ` Palmer Dabbelt
2021-06-21  7:11               ` Alex Ghiti
2022-11-10 21:01                 ` Dmitry Vyukov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).