Re: [PATCH] riscv: Bump COMMAND_LINE_SIZE value to 1024
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
Re: [PATCH] riscv: Bump COMMAND_LINE_SIZE value to 1024
On Fri, Apr 2, 2021 at 11:43 AM Dmitry Vyukov wrote: > > On Fri, Apr 2, 2021 at 6:37 AM Palmer Dabbelt wrote: > > > > On Tue, 30 Mar 2021 13:31:45 PDT (-0700), ma...@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
Re: [PATCH] riscv: Bump COMMAND_LINE_SIZE value to 1024
On Fri, Apr 2, 2021 at 6:37 AM Palmer Dabbelt wrote: > > On Tue, 30 Mar 2021 13:31:45 PDT (-0700), ma...@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?
Re: [PATCH] riscv: Bump COMMAND_LINE_SIZE value to 1024
On Tue, 30 Mar 2021 13:31:45 PDT (-0700), ma...@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.
Re: [PATCH] riscv: Bump COMMAND_LINE_SIZE value to 1024
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
Re: [PATCH] riscv: Bump COMMAND_LINE_SIZE value to 1024
On Tue, 16 Mar 2021 12:34:20 PDT (-0700), a...@ghiti.fr wrote: Increase COMMAND_LINE_SIZE as the current default value is too low for syzbot kernel command line. Reported-by: Dmitry Vyukov Signed-off-by: Alexandre Ghiti --- 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 ..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.
[PATCH] riscv: Bump COMMAND_LINE_SIZE value to 1024
Increase COMMAND_LINE_SIZE as the current default value is too low for syzbot kernel command line. Reported-by: Dmitry Vyukov Signed-off-by: Alexandre Ghiti --- 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 ..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
