[PATCH 0/3] Three small fixes to cmdline parsing

[PATCH 0/3] Three small fixes to cmdline parsing

[Arvind Sankar]

First 2 are resends:
https://lore.kernel.org/linux-efi/20200725155916.1376773-1-nivedita@alum.mit.edu/
https://lore.kernel.org/linux-efi/20200729193300.598448-1-nivedita@alum.mit.edu/

Arvind Sankar (3):
  efi/libstub: Stop parsing arguments at "--"
  efi/libstub: Handle NULL cmdline
  efi/libstub: Handle unterminated cmdline

 drivers/firmware/efi/libstub/efi-stub-helper.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

-- 
2.26.2

[PATCH 1/3] efi/libstub: Stop parsing arguments at "--"

[Arvind Sankar]

Arguments after "--" are arguments for init, not for the kernel.

Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
---
 drivers/firmware/efi/libstub/efi-stub-helper.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c
index 6bca70bbb43d..37ff34e7b85e 100644
--- a/drivers/firmware/efi/libstub/efi-stub-helper.c
+++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
@@ -201,6 +201,8 @@ efi_status_t efi_parse_options(char const *cmdline)
 		char *param, *val;
 
 		str = next_arg(str, &param, &val);
+		if (!val && !strcmp(param, "--"))
+			break;
 
 		if (!strcmp(param, "nokaslr")) {
 			efi_nokaslr = true;
-- 
2.26.2

[PATCH 2/3] efi/libstub: Handle NULL cmdline

[Arvind Sankar]

Treat a NULL cmdline the same as empty. Although this is unlikely to
happen in practice, the x86 kernel entry does check for NULL cmdline and
handles it, so do it here as well.

Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
---
 drivers/firmware/efi/libstub/efi-stub-helper.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c
index 37ff34e7b85e..f53652a3a106 100644
--- a/drivers/firmware/efi/libstub/efi-stub-helper.c
+++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
@@ -187,10 +187,14 @@ int efi_printk(const char *fmt, ...)
  */
 efi_status_t efi_parse_options(char const *cmdline)
 {
-	size_t len = strlen(cmdline) + 1;
+	size_t len;
 	efi_status_t status;
 	char *str, *buf;
 
+	if (!cmdline)
+		return EFI_SUCCESS;
+
+	len = strlen(cmdline) + 1;
 	status = efi_bs_call(allocate_pool, EFI_LOADER_DATA, len, (void **)&buf);
 	if (status != EFI_SUCCESS)
 		return status;
-- 
2.26.2

[PATCH 3/3] efi/libstub: Handle unterminated cmdline

[Arvind Sankar]

Make the command line parsing more robust, by handling the case it is
not NUL-terminated.

Use strnlen instead of strlen, and make sure that the temporary copy is
NUL-terminated before parsing.

Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
---
 drivers/firmware/efi/libstub/efi-stub-helper.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c
index f53652a3a106..fe5103086e27 100644
--- a/drivers/firmware/efi/libstub/efi-stub-helper.c
+++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
@@ -194,12 +194,14 @@ efi_status_t efi_parse_options(char const *cmdline)
 	if (!cmdline)
 		return EFI_SUCCESS;
 
-	len = strlen(cmdline) + 1;
+	len = strnlen(cmdline, COMMAND_LINE_SIZE-1) + 1;
 	status = efi_bs_call(allocate_pool, EFI_LOADER_DATA, len, (void **)&buf);
 	if (status != EFI_SUCCESS)
 		return status;
 
-	str = skip_spaces(memcpy(buf, cmdline, len));
+	memcpy(buf, cmdline, len-1);
+	buf[len-1] = '\0';
+	str = skip_spaces(buf);
 
 	while (*str) {
 		char *param, *val;
-- 
2.26.2

[tip: efi/urgent] efi/libstub: Handle unterminated cmdline

[tip-bot2 for Arvind Sankar]

The following commit has been merged into the efi/urgent branch of tip:

Commit-ID:     8a8a3237a78cbc0557f0eb16a89f16d616323e99
Gitweb:        https://git.kernel.org/tip/8a8a3237a78cbc0557f0eb16a89f16d616323e99
Author:        Arvind Sankar <nivedita@alum.mit.edu>
AuthorDate:    Thu, 13 Aug 2020 14:58:11 -04:00
Committer:     Ard Biesheuvel <ardb@kernel.org>
CommitterDate: Thu, 20 Aug 2020 11:18:58 +02:00

efi/libstub: Handle unterminated cmdline

Make the command line parsing more robust, by handling the case it is
not NUL-terminated.

Use strnlen instead of strlen, and make sure that the temporary copy is
NUL-terminated before parsing.

Cc: <stable@vger.kernel.org>
Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
Link: https://lore.kernel.org/r/20200813185811.554051-4-nivedita@alum.mit.edu
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/firmware/efi/libstub/efi-stub-helper.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c
index f53652a..f735db5 100644
--- a/drivers/firmware/efi/libstub/efi-stub-helper.c
+++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
@@ -194,12 +194,14 @@ efi_status_t efi_parse_options(char const *cmdline)
 	if (!cmdline)
 		return EFI_SUCCESS;
 
-	len = strlen(cmdline) + 1;
+	len = strnlen(cmdline, COMMAND_LINE_SIZE - 1) + 1;
 	status = efi_bs_call(allocate_pool, EFI_LOADER_DATA, len, (void **)&buf);
 	if (status != EFI_SUCCESS)
 		return status;
 
-	str = skip_spaces(memcpy(buf, cmdline, len));
+	memcpy(buf, cmdline, len - 1);
+	buf[len - 1] = '\0';
+	str = skip_spaces(buf);
 
 	while (*str) {
 		char *param, *val;

Re: [PATCH 0/3] Three small fixes to cmdline parsing

[Ard Biesheuvel]

On Thu, 13 Aug 2020 at 20:58, Arvind Sankar <nivedita@alum.mit.edu> wrote:
>
> First 2 are resends:
> https://lore.kernel.org/linux-efi/20200725155916.1376773-1-nivedita@alum.mit.edu/
> https://lore.kernel.org/linux-efi/20200729193300.598448-1-nivedita@alum.mit.edu/
>
> Arvind Sankar (3):
>   efi/libstub: Stop parsing arguments at "--"
>   efi/libstub: Handle NULL cmdline
>   efi/libstub: Handle unterminated cmdline
>

Thanks Arvind. I already applied #1 and #2 locally, so I will just
pick up the last one.


>  drivers/firmware/efi/libstub/efi-stub-helper.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
>
> --
> 2.26.2
>