[PATCH] riscv: Bump COMMAND_LINE_SIZE value to 1024

Subsystems: risc-v architecture, the rest

STALE1250d LANDED

Landed in mainline as 61fc1ee8be26 on 2023-03-02.

13 messages, 5 authors, 2023-03-02 · open the first message on its own page

[PATCH] riscv: Bump COMMAND_LINE_SIZE value to 1024

From: Alexandre Ghiti <alex@ghiti.fr>
Date: 2021-03-16 19:35:37 

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

Re: [PATCH] riscv: Bump COMMAND_LINE_SIZE value to 1024

From: Palmer Dabbelt <palmer@dabbelt.com>
Date: 2021-03-30 05:08:53 

On Tue, 16 Mar 2021 12:34:20 PDT (-0700), alex@ghiti.fr wrote:
quoted hunk
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.

Re: [PATCH] riscv: Bump COMMAND_LINE_SIZE value to 1024

From: "Maciej W. Rozycki" <macro@orcam.me.uk>
Date: 2021-03-30 20:32:32 

On Mon, 29 Mar 2021, Palmer Dabbelt wrote:
quoted
--- /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

Re: [PATCH] riscv: Bump COMMAND_LINE_SIZE value to 1024

From: Palmer Dabbelt <palmer@dabbelt.com>
Date: 2021-04-02 04:37:19 

On Tue, 30 Mar 2021 13:31:45 PDT (-0700), macro@orcam.me.uk wrote:
On Mon, 29 Mar 2021, Palmer Dabbelt wrote:
quoted
quoted
--- /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.

Re: [PATCH] riscv: Bump COMMAND_LINE_SIZE value to 1024

From: Dmitry Vyukov <dvyukov@google.com>
Date: 2021-04-02 08:41:18 

On Fri, Apr 2, 2021 at 6:37 AM Palmer Dabbelt [off-list ref] wrote:
On Tue, 30 Mar 2021 13:31:45 PDT (-0700), macro@orcam.me.uk wrote:
quoted
On Mon, 29 Mar 2021, Palmer Dabbelt wrote:
quoted
quoted
--- /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?

Re: [PATCH] riscv: Bump COMMAND_LINE_SIZE value to 1024

From: David Abdurachmanov <hidden>
Date: 2021-04-02 08:59:32 

On Fri, Apr 2, 2021 at 11:43 AM Dmitry Vyukov [off-list ref] wrote:
On Fri, Apr 2, 2021 at 6:37 AM Palmer Dabbelt [off-list ref] wrote:
quoted
On Tue, 30 Mar 2021 13:31:45 PDT (-0700), macro@orcam.me.uk wrote:
quoted
On Mon, 29 Mar 2021, Palmer Dabbelt wrote:
quoted
quoted
--- /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

Re: [PATCH] riscv: Bump COMMAND_LINE_SIZE value to 1024

From: "Maciej W. Rozycki" <macro@orcam.me.uk>
Date: 2021-04-02 18:33:35 

On Fri, 2 Apr 2021, David Abdurachmanov wrote:
quoted
quoted
quoted
 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

Re: [PATCH] riscv: Bump COMMAND_LINE_SIZE value to 1024

From: Palmer Dabbelt <palmer@dabbelt.com>
Date: 2021-04-23 02:57:43 

On Fri, 02 Apr 2021 11:33:30 PDT (-0700), macro@orcam.me.uk wrote:
On Fri, 2 Apr 2021, David Abdurachmanov wrote:
quoted
quoted
quoted
quoted
 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

Re: [PATCH] riscv: Bump COMMAND_LINE_SIZE value to 1024

From: Alex Ghiti <alex@ghiti.fr>
Date: 2021-06-21 07:11:43 

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:
quoted
On Fri, 2 Apr 2021, David Abdurachmanov wrote:
quoted
quoted
quoted
quoted
 This macro is exported as a part of the user API so it must 
not depend on
quoted
quoted
quoted
Kconfig.  Also changing it (rather than say adding 
COMMAND_LINE_SIZE_V2 or
quoted
quoted
quoted
switching to an entirely new data object that has its dimension 
set in a
quoted
quoted
quoted
different way) requires careful evaluation as external binaries 
have and
quoted
quoted
quoted
will have the value it expands to compiled in, so it's a part 
of the ABI
quoted
quoted
quoted
too.
Thanks, I didn't realize this was part of the user BI.  In that 
case we
quoted
quoted
really can't chage it, so we'll have to sort out some other way 
do fix
quoted
quoted
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

Re: [PATCH] riscv: Bump COMMAND_LINE_SIZE value to 1024

From: Dmitry Vyukov <dvyukov@google.com>
Date: 2022-11-10 21:01:45 

On Mon, 21 Jun 2021 at 00:11, Alex Ghiti [off-list ref] wrote:
Hi Palmer,

Le 23/04/2021 à 04:57, Palmer Dabbelt a écrit :
quoted
On Fri, 02 Apr 2021 11:33:30 PDT (-0700), macro@orcam.me.uk wrote:
quoted
On Fri, 2 Apr 2021, David Abdurachmanov wrote:
quoted
quoted
quoted
quoted
 This macro is exported as a part of the user API so it must
not depend on
quoted
quoted
quoted
Kconfig.  Also changing it (rather than say adding
COMMAND_LINE_SIZE_V2 or
quoted
quoted
quoted
switching to an entirely new data object that has its dimension
set in a
quoted
quoted
quoted
different way) requires careful evaluation as external binaries
have and
quoted
quoted
quoted
will have the value it expands to compiled in, so it's a part
of the ABI
quoted
quoted
quoted
too.
Thanks, I didn't realize this was part of the user BI.  In that
case we
quoted
quoted
really can't chage it, so we'll have to sort out some other way
do fix
quoted
quoted
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.

Re: [PATCH] riscv: Bump COMMAND_LINE_SIZE value to 1024

From: Dmitry Vyukov <dvyukov@google.com>
Date: 2023-02-09 11:51:23 

On Thu, 10 Nov 2022 at 22:01, Dmitry Vyukov [off-list ref] wrote:
On Mon, 21 Jun 2021 at 00:11, Alex Ghiti [off-list ref] wrote:
quoted
Hi Palmer,

Le 23/04/2021 à 04:57, Palmer Dabbelt a écrit :
quoted
On Fri, 02 Apr 2021 11:33:30 PDT (-0700), macro@orcam.me.uk wrote:
quoted
On Fri, 2 Apr 2021, David Abdurachmanov wrote:
quoted
quoted
quoted
quoted
 This macro is exported as a part of the user API so it must
not depend on
quoted
quoted
quoted
Kconfig.  Also changing it (rather than say adding
COMMAND_LINE_SIZE_V2 or
quoted
quoted
quoted
switching to an entirely new data object that has its dimension
set in a
quoted
quoted
quoted
different way) requires careful evaluation as external binaries
have and
quoted
quoted
quoted
will have the value it expands to compiled in, so it's a part
of the ABI
quoted
quoted
quoted
too.
Thanks, I didn't realize this was part of the user BI.  In that
case we
quoted
quoted
really can't chage it, so we'll have to sort out some other way
do fix
quoted
quoted
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.
Ping. Still an issue at least for syzbot.

Re: [PATCH] riscv: Bump COMMAND_LINE_SIZE value to 1024

From: Alex Ghiti <alex@ghiti.fr>
Date: 2023-02-09 19:30:22 

Le 9/02/2023 à 12:37, Dmitry Vyukov a écrit :
On Thu, 10 Nov 2022 at 22:01, Dmitry Vyukov [off-list ref] wrote:
quoted
On Mon, 21 Jun 2021 at 00:11, Alex Ghiti [off-list ref] wrote:
quoted
Hi Palmer,

Le 23/04/2021 à 04:57, Palmer Dabbelt a écrit :
quoted
On Fri, 02 Apr 2021 11:33:30 PDT (-0700), macro@orcam.me.uk wrote:
quoted
On Fri, 2 Apr 2021, David Abdurachmanov wrote:
quoted
quoted
quoted
quoted
  This macro is exported as a part of the user API so it must
not depend on
quoted
quoted
quoted
Kconfig.  Also changing it (rather than say adding
COMMAND_LINE_SIZE_V2 or
quoted
quoted
quoted
switching to an entirely new data object that has its dimension
set in a
quoted
quoted
quoted
different way) requires careful evaluation as external binaries
have and
quoted
quoted
quoted
will have the value it expands to compiled in, so it's a part
of the ABI
quoted
quoted
quoted
too.
Thanks, I didn't realize this was part of the user BI.  In that
case we
quoted
quoted
really can't chage it, so we'll have to sort out some other way
do fix
quoted
quoted
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.
Ping. Still an issue at least for syzbot.
Yes, agreed, Palmer proposed the following instead since blindly 
increasing the command line size would break userspace ABI: 
https://lore.kernel.org/lkml/20221211061358.28035-1-palmer@rivosinc.com/T/

I will bump this thread to make progress, thanks for the ping.

Alex

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

Re: [PATCH] riscv: Bump COMMAND_LINE_SIZE value to 1024

From: Palmer Dabbelt <palmer@dabbelt.com>
Date: 2023-03-02 03:17:22 

On Tue, 16 Mar 2021 12:34:20 PDT (-0700), alex@ghiti.fr wrote:
quoted hunk
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 */
This is now back on for-next, with some commit text explaining why it's 
not a uABI change as per Arnd's message 
<https://lore.kernel.org/linux-riscv/874b8076-b0d1-4aaa-bcd8-05d523060152@app.fastmail.com/#t>.  
I intend on sending this one for 6.3 as syzkaller depends on it.

Thanks!
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help