[PATCH v3 0/5] Implement fastboot multiresponce

Svyatoslav Ryhel

21 Nov 2023 21 Nov '23

7:35 p.m.

Currently u-boot fastboot can only send one message back to host, so if there is a need to print more than one line messages must be kept sending until all the required data is obtained. This behavior can be adjusted using multiresponce ability (getting multiple lines of response) proposed in this patch set.

Implementation of multiresponce leads to ability to dump content of console buffer which, with use of "oem run", allows to entirely avoid need in UART.

--- Changes from v2: - fix missing function calls if CONFIG_CONSOLE_RECORD is not enabled

Changes from v1: - changed variables to static - fixed multiresponce for udp - documented use of "MORE" - converted #if to if (...) ---

Ion Agorria (5): fastboot: multiresponse support fastboot: implement "getvar all" common: console: introduce overflow and isempty calls lib: membuff: fix readline not returning line in case of overflow fastboot: add oem console command support

-- 2.40.1

Show replies by date

Svyatoslav Ryhel

21 Nov 21 Nov

7:35 p.m.

New subject: [PATCH v3 1/5] fastboot: multiresponse support

From: Ion Agorria ion@agorria.com

Signed-off-by: Ion Agorria ion@agorria.com Signed-off-by: Svyatoslav Ryhel clamor95@gmail.com Reviewed-by: Mattijs Korpershoek mkorpershoek@baylibre.com --- drivers/fastboot/fb_command.c | 10 ++++++++++ drivers/usb/gadget/f_fastboot.c | 29 +++++++++++++++++++++++++++++ include/fastboot.h | 18 ++++++++++++++++++ net/fastboot_udp.c | 29 +++++++++++++++++++++++------ 4 files changed, 80 insertions(+), 6 deletions(-)

diff --git a/drivers/fastboot/fb_command.c b/drivers/fastboot/fb_command.c index 5fcadcdf50..ab72d8c781 100644 --- a/drivers/fastboot/fb_command.c +++ b/drivers/fastboot/fb_command.c @@ -5,6 +5,7 @@

#include <common.h> #include <command.h> +#include <console.h> #include <env.h> #include <fastboot.h> #include <fastboot-internal.h> @@ -152,6 +153,15 @@ int fastboot_handle_command(char *cmd_string, char *response) return -1; }

+void fastboot_multiresponse(int cmd, char *response) +{ + switch (cmd) { + default: + fastboot_fail("Unknown multiresponse command", response); + break; + } +} + /** * okay() - Send bare OKAY response * diff --git a/drivers/usb/gadget/f_fastboot.c b/drivers/usb/gadget/f_fastboot.c index 741775a7bc..0850350331 100644 --- a/drivers/usb/gadget/f_fastboot.c +++ b/drivers/usb/gadget/f_fastboot.c @@ -497,6 +497,25 @@ static void do_bootm_on_complete(struct usb_ep *ep, struct usb_request *req) do_exit_on_complete(ep, req); }

+static int multiresponse_cmd = -1; +static void multiresponse_on_complete(struct usb_ep *ep, struct usb_request *req) +{ + char response[FASTBOOT_RESPONSE_LEN] = {0}; + + if (multiresponse_cmd == -1) + return; + + /* Call handler to obtain next response */ + fastboot_multiresponse(multiresponse_cmd, response); + fastboot_tx_write_str(response); + + /* If response is final OKAY/FAIL response disconnect this handler and unset cmd */ + if (!strncmp("OKAY", response, 4) || !strncmp("FAIL", response, 4)) { + multiresponse_cmd = -1; + fastboot_func->in_req->complete = fastboot_complete; + } +} + static void do_acmd_complete(struct usb_ep *ep, struct usb_request *req) { /* When usb dequeue complete will be called @@ -524,6 +543,16 @@ static void rx_handler_command(struct usb_ep *ep, struct usb_request *req) fastboot_fail("buffer overflow", response); }

+ if (!strncmp(FASTBOOT_MULTIRESPONSE_START, response, 4)) { + multiresponse_cmd = cmd; + fastboot_multiresponse(multiresponse_cmd, response); + + /* Only add complete callback if first is not a final OKAY/FAIL response */ + if (strncmp("OKAY", response, 4) && strncmp("FAIL", response, 4)) { + fastboot_func->in_req->complete = multiresponse_on_complete; + } + } + if (!strncmp("DATA", response, 4)) { req->complete = rx_handler_dl_image; req->length = rx_bytes_expected(ep); diff --git a/include/fastboot.h b/include/fastboot.h index 296451f89d..59cbea61ec 100644 --- a/include/fastboot.h +++ b/include/fastboot.h @@ -14,6 +14,16 @@

#define FASTBOOT_VERSION "0.4"

+/* + * Signals u-boot fastboot code to send multiple responses by + * calling response generating function repeatedly until a OKAY/FAIL + * is generated as final response. + * + * This status code is only used internally to signal, must NOT + * be sent to host. + */ +#define FASTBOOT_MULTIRESPONSE_START ("MORE") + /* The 64 defined bytes plus \0 */ #define FASTBOOT_COMMAND_LEN (64 + 1) #define FASTBOOT_RESPONSE_LEN (64 + 1) @@ -172,5 +182,13 @@ void fastboot_data_download(const void *fastboot_data, */ void fastboot_data_complete(char *response);

+/** + * fastboot_handle_multiresponse() - Called for each response to send + * + * @cmd: Command id that requested multiresponse + * @response: Pointer to fastboot response buffer + */ +void fastboot_multiresponse(int cmd, char *response); + void fastboot_acmd_complete(void); #endif /* _FASTBOOT_H_ */ diff --git a/net/fastboot_udp.c b/net/fastboot_udp.c index d690787478..6fee441ab3 100644 --- a/net/fastboot_udp.c +++ b/net/fastboot_udp.c @@ -42,16 +42,15 @@ static int fastboot_remote_port; static int fastboot_our_port;

/** - * fastboot_udp_send_info() - Send an INFO packet during long commands. + * fastboot_udp_send_response() - Send an response into UDP * - * @msg: String describing the reason for waiting + * @response: Response to send */ -static void fastboot_udp_send_info(const char *msg) +static void fastboot_udp_send_response(const char *response) { uchar *packet; uchar *packet_base; int len = 0; - char response[FASTBOOT_RESPONSE_LEN] = {0};

struct fastboot_header response_header = { .id = FASTBOOT_FASTBOOT, @@ -66,7 +65,6 @@ static void fastboot_udp_send_info(const char *msg) memcpy(packet, &response_header, sizeof(response_header)); packet += sizeof(response_header); /* Write response */ - fastboot_response("INFO", response, "%s", msg); memcpy(packet, response, strlen(response)); packet += strlen(response);

@@ -91,6 +89,7 @@ static void fastboot_udp_send_info(const char *msg) static void fastboot_timed_send_info(const char *msg) { static ulong start; + char response[FASTBOOT_RESPONSE_LEN] = {0};

/* Initialize timer */ if (start == 0) @@ -99,7 +98,8 @@ static void fastboot_timed_send_info(const char *msg) /* Send INFO packet to host every 30 seconds */ if (time >= 30000) { start = get_timer(0); - fastboot_udp_send_info(msg); + fastboot_response("INFO", response, "%s", msg); + fastboot_udp_send_response(response); } }

@@ -180,6 +180,23 @@ static void fastboot_send(struct fastboot_header header, char *fastboot_data, } else { cmd = fastboot_handle_command(command, response); pending_command = false; + + if (!strncmp(FASTBOOT_MULTIRESPONSE_START, response, 4)) { + while (1) { + /* Call handler to obtain next response */ + fastboot_multiresponse(cmd, response); + + /* + * Send more responses or break to send + * final OKAY/FAIL response + */ + if (strncmp("OKAY", response, 4) && + strncmp("FAIL", response, 4)) + fastboot_udp_send_response(response); + else + break; + } + } } /* * Sent some INFO packets, need to update sequence number in

-- 2.40.1

Svyatoslav Ryhel

7:35 p.m.

New subject: [PATCH v3 2/5] fastboot: implement "getvar all"

From: Ion Agorria ion@agorria.com

This commit implements "fastboot getvar all" listing by iterating the existing dispatchers that don't require parameters (as we pass NULL), uses fastboot multiresponse.

Signed-off-by: Ion Agorria ion@agorria.com Signed-off-by: Svyatoslav Ryhel clamor95@gmail.com Reviewed-by: Mattijs Korpershoek mkorpershoek@baylibre.com --- doc/android/fastboot-protocol.rst | 3 ++ drivers/fastboot/fb_command.c | 3 ++ drivers/fastboot/fb_getvar.c | 77 +++++++++++++++++++++++++------ include/fastboot-internal.h | 7 +++ 4 files changed, 77 insertions(+), 13 deletions(-)

diff --git a/doc/android/fastboot-protocol.rst b/doc/android/fastboot-protocol.rst index e8cbd7f24e..8bd6d7168f 100644 --- a/doc/android/fastboot-protocol.rst +++ b/doc/android/fastboot-protocol.rst @@ -173,6 +173,9 @@ The various currently defined names are:: bootloader requiring a signature before it will install or boot images.

+ all Provides all info from commands above as + they were called one by one + Names starting with a lowercase character are reserved by this specification. OEM-specific names should not start with lowercase characters. diff --git a/drivers/fastboot/fb_command.c b/drivers/fastboot/fb_command.c index ab72d8c781..6f621df074 100644 --- a/drivers/fastboot/fb_command.c +++ b/drivers/fastboot/fb_command.c @@ -156,6 +156,9 @@ int fastboot_handle_command(char *cmd_string, char *response) void fastboot_multiresponse(int cmd, char *response) { switch (cmd) { + case FASTBOOT_COMMAND_GETVAR: + fastboot_getvar_all(response); + break; default: fastboot_fail("Unknown multiresponse command", response); break; diff --git a/drivers/fastboot/fb_getvar.c b/drivers/fastboot/fb_getvar.c index 8cb8ffa2c6..f65519c57b 100644 --- a/drivers/fastboot/fb_getvar.c +++ b/drivers/fastboot/fb_getvar.c @@ -29,53 +29,67 @@ static void getvar_is_userspace(char *var_parameter, char *response);

static const struct { const char *variable; + bool list; void (*dispatch)(char *var_parameter, char *response); } getvar_dispatch[] = { { .variable = "version", - .dispatch = getvar_version + .dispatch = getvar_version, + .list = true, }, { .variable = "version-bootloader", - .dispatch = getvar_version_bootloader + .dispatch = getvar_version_bootloader, + .list = true }, { .variable = "downloadsize", - .dispatch = getvar_downloadsize + .dispatch = getvar_downloadsize, + .list = true }, { .variable = "max-download-size", - .dispatch = getvar_downloadsize + .dispatch = getvar_downloadsize, + .list = true }, { .variable = "serialno", - .dispatch = getvar_serialno + .dispatch = getvar_serialno, + .list = true }, { .variable = "version-baseband", - .dispatch = getvar_version_baseband + .dispatch = getvar_version_baseband, + .list = true }, { .variable = "product", - .dispatch = getvar_product + .dispatch = getvar_product, + .list = true }, { .variable = "platform", - .dispatch = getvar_platform + .dispatch = getvar_platform, + .list = true }, { .variable = "current-slot", - .dispatch = getvar_current_slot + .dispatch = getvar_current_slot, + .list = true #if IS_ENABLED(CONFIG_FASTBOOT_FLASH) }, { .variable = "has-slot", - .dispatch = getvar_has_slot + .dispatch = getvar_has_slot, + .list = false #endif #if IS_ENABLED(CONFIG_FASTBOOT_FLASH_MMC) }, { .variable = "partition-type", - .dispatch = getvar_partition_type + .dispatch = getvar_partition_type, + .list = false #endif #if IS_ENABLED(CONFIG_FASTBOOT_FLASH) }, { .variable = "partition-size", - .dispatch = getvar_partition_size + .dispatch = getvar_partition_size, + .list = false #endif }, { .variable = "is-userspace", - .dispatch = getvar_is_userspace + .dispatch = getvar_is_userspace, + .list = true } };

@@ -237,6 +251,40 @@ static void getvar_is_userspace(char *var_parameter, char *response) fastboot_okay("no", response); }

+static int current_all_dispatch; +void fastboot_getvar_all(char *response) +{ + /* + * Find a dispatch getvar that can be listed and send + * it as INFO until we reach the end. + */ + while (current_all_dispatch < ARRAY_SIZE(getvar_dispatch)) { + if (!getvar_dispatch[current_all_dispatch].list) { + current_all_dispatch++; + continue; + } + + char envstr[FASTBOOT_RESPONSE_LEN] = { 0 }; + + getvar_dispatch[current_all_dispatch].dispatch(NULL, envstr); + + char *envstr_start = envstr; + + if (!strncmp("OKAY", envstr, 4) || !strncmp("FAIL", envstr, 4)) + envstr_start += 4; + + fastboot_response("INFO", response, "%s: %s", + getvar_dispatch[current_all_dispatch].variable, + envstr_start); + + current_all_dispatch++; + return; + } + + fastboot_response("OKAY", response, NULL); + current_all_dispatch = 0; +} + /** * fastboot_getvar() - Writes variable indicated by cmd_parameter to response. * @@ -254,6 +302,9 @@ void fastboot_getvar(char *cmd_parameter, char *response) { if (!cmd_parameter) { fastboot_fail("missing var", response); + } else if (!strncmp("all", cmd_parameter, 3) && strlen(cmd_parameter) == 3) { + current_all_dispatch = 0; + fastboot_response(FASTBOOT_MULTIRESPONSE_START, response, NULL); } else { #define FASTBOOT_ENV_PREFIX "fastboot." int i; diff --git a/include/fastboot-internal.h b/include/fastboot-internal.h index bf2f2b3c89..610d4f9141 100644 --- a/include/fastboot-internal.h +++ b/include/fastboot-internal.h @@ -18,6 +18,13 @@ extern u32 fastboot_buf_size; */ extern void (*fastboot_progress_callback)(const char *msg);

+/** + * fastboot_getvar_all() - Writes current variable being listed from "all" to response. + * + * @response: Pointer to fastboot response buffer + */ +void fastboot_getvar_all(char *response); + /** * fastboot_getvar() - Writes variable indicated by cmd_parameter to response. *

-- 2.40.1

Svyatoslav Ryhel

7:35 p.m.

New subject: [PATCH v3 3/5] common: console: introduce overflow and isempty calls

From: Ion Agorria ion@agorria.com

Separate record overflow logic and add console_record_isempty as available calls don't serve to know output has been read fully with readline's.

Signed-off-by: Ion Agorria ion@agorria.com Signed-off-by: Svyatoslav Ryhel clamor95@gmail.com Reviewed-by: Mattijs Korpershoek mkorpershoek@baylibre.com --- common/console.c | 15 ++++++++++++--- include/console.h | 26 ++++++++++++++++++++++++++ test/ut.c | 9 ++++----- 3 files changed, 42 insertions(+), 8 deletions(-)

diff --git a/common/console.c b/common/console.c index 98c3ee6ca6..8a869b137e 100644 --- a/common/console.c +++ b/common/console.c @@ -818,6 +818,8 @@ int console_record_init(void) ret = membuff_new((struct membuff *)&gd->console_in, CONFIG_CONSOLE_RECORD_IN_SIZE);

+ gd->flags |= GD_FLG_RECORD; + return ret; }

@@ -836,11 +838,13 @@ int console_record_reset_enable(void) return 0; }

-int console_record_readline(char *str, int maxlen) +bool console_record_overflow(void) { - if (gd->flags & GD_FLG_RECORD_OVF) - return -ENOSPC; + return gd->flags & GD_FLG_RECORD_OVF ? true : false; +}

+int console_record_readline(char *str, int maxlen) +{ return membuff_readline((struct membuff *)&gd->console_out, str, maxlen, '\0'); } @@ -850,6 +854,11 @@ int console_record_avail(void) return membuff_avail((struct membuff *)&gd->console_out); }

+bool console_record_isempty(void) +{ + return membuff_isempty((struct membuff *)&gd->console_out); +} + int console_in_puts(const char *str) { return membuff_put((struct membuff *)&gd->console_in, str, strlen(str)); diff --git a/include/console.h b/include/console.h index ceb733b5cb..d949d29c13 100644 --- a/include/console.h +++ b/include/console.h @@ -64,6 +64,13 @@ void console_record_reset(void); */ int console_record_reset_enable(void);

+/** + * console_record_overflow() - returns state of buffers overflow + * + * Return: true if the console buffer was overflowed + */ +bool console_record_overflow(void); + /** * console_record_readline() - Read a line from the console output * @@ -84,6 +91,13 @@ int console_record_readline(char *str, int maxlen); */ int console_record_avail(void);

+/** + * console_record_isempty() - Returns if console output is empty + * + * Return: true if empty + */ +bool console_record_isempty(void); + /** * console_in_puts() - Write a string to the console input buffer * @@ -113,6 +127,12 @@ static inline int console_record_reset_enable(void) return -ENOSYS; }

+static inline bool console_record_overflow(void) +{ + /* No overflow */ + return false; +} + static inline int console_record_readline(char *str, int maxlen) { /* Nothing to read */ @@ -131,6 +151,12 @@ static inline int console_in_puts(const char *str) return 0; }

+static inline bool console_record_isempty(void) +{ + /* Always empty */ + return true; +} + #endif /* !CONFIG_CONSOLE_RECORD */

/** diff --git a/test/ut.c b/test/ut.c index 28da417686..d202644a15 100644 --- a/test/ut.c +++ b/test/ut.c @@ -53,15 +53,14 @@ long ut_check_delta(ulong last)

static int readline_check(struct unit_test_state *uts) { - int ret; - - ret = console_record_readline(uts->actual_str, sizeof(uts->actual_str)); - if (ret == -ENOSPC) { + if (console_record_overflow()) { ut_fail(uts, __FILE__, __LINE__, __func__, "Console record buffer too small - increase CONFIG_CONSOLE_RECORD_OUT_SIZE"); - return ret; + return -ENOSPC; }

+ console_record_readline(uts->actual_str, sizeof(uts->actual_str)); + return 0; }

-- 2.40.1

Svyatoslav Ryhel

7:35 p.m.

New subject: [PATCH v3 4/5] lib: membuff: fix readline not returning line in case of overflow

From: Ion Agorria ion@agorria.com

If line overflows readline it will not be returned, fix this behavior, make it optional and documented properly.

Signed-off-by: Ion Agorria ion@agorria.com Signed-off-by: Svyatoslav Ryhel clamor95@gmail.com Reviewed-by: Mattijs Korpershoek mkorpershoek@baylibre.com --- boot/bootmeth_extlinux.c | 2 +- common/console.c | 2 +- include/membuff.h | 5 +++-- lib/membuff.c | 4 ++-- 4 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/boot/bootmeth_extlinux.c b/boot/bootmeth_extlinux.c index aa2a4591eb..ae0ad1d53e 100644 --- a/boot/bootmeth_extlinux.c +++ b/boot/bootmeth_extlinux.c @@ -82,7 +82,7 @@ static int extlinux_fill_info(struct bootflow *bflow) log_debug("parsing bflow file size %x\n", bflow->size); membuff_init(&mb, bflow->buf, bflow->size); membuff_putraw(&mb, bflow->size, true, &data); - while (len = membuff_readline(&mb, line, sizeof(line) - 1, ' '), len) { + while (len = membuff_readline(&mb, line, sizeof(line) - 1, ' ', true), len) { char *tok, *p = line;

tok = strsep(&p, " "); diff --git a/common/console.c b/common/console.c index 8a869b137e..cd56035171 100644 --- a/common/console.c +++ b/common/console.c @@ -846,7 +846,7 @@ bool console_record_overflow(void) int console_record_readline(char *str, int maxlen) { return membuff_readline((struct membuff *)&gd->console_out, str, - maxlen, '\0'); + maxlen, '\0', false); }

int console_record_avail(void) diff --git a/include/membuff.h b/include/membuff.h index 21051b0c54..4eba626ce1 100644 --- a/include/membuff.h +++ b/include/membuff.h @@ -192,10 +192,11 @@ int membuff_free(struct membuff *mb); * @mb: membuff to adjust * @str: Place to put the line * @maxlen: Maximum line length (excluding terminator) + * @must_fit: If true then str is empty if line doesn't fit * Return: number of bytes read (including terminator) if a line has been - * read, 0 if nothing was there + * read, 0 if nothing was there or line didn't fit when must_fit is set */ -int membuff_readline(struct membuff *mb, char *str, int maxlen, int minch); +int membuff_readline(struct membuff *mb, char *str, int maxlen, int minch, bool must_fit);

/** * membuff_extend_by() - expand a membuff diff --git a/lib/membuff.c b/lib/membuff.c index 36dc43a523..964e504d5b 100644 --- a/lib/membuff.c +++ b/lib/membuff.c @@ -288,7 +288,7 @@ int membuff_free(struct membuff *mb) (mb->end - mb->start) - 1 - membuff_avail(mb); }

-int membuff_readline(struct membuff *mb, char *str, int maxlen, int minch) +int membuff_readline(struct membuff *mb, char *str, int maxlen, int minch, bool must_fit) { int len; /* number of bytes read (!= string length) */ char *s, *end; @@ -310,7 +310,7 @@ int membuff_readline(struct membuff *mb, char *str, int maxlen, int minch) }

/* couldn't get the whole string */ - if (!ok) { + if (!ok && must_fit) { if (maxlen) *orig = '\0'; return 0;

-- 2.40.1

Simon Glass

7:40 p.m.

New subject: [PATCH v3 4/5] lib: membuff: fix readline not returning line in case of overflow

Hi Svyatoslav,

On Tue, 21 Nov 2023 at 11:35, Svyatoslav Ryhel clamor95@gmail.com wrote:

...

From: Ion Agorria ion@agorria.com

If line overflows readline it will not be returned, fix this behavior, make it optional and documented properly.

Signed-off-by: Ion Agorria ion@agorria.com Signed-off-by: Svyatoslav Ryhel clamor95@gmail.com Reviewed-by: Mattijs Korpershoek mkorpershoek@baylibre.com

boot/bootmeth_extlinux.c | 2 +- common/console.c | 2 +- include/membuff.h | 5 +++-- lib/membuff.c | 4 ++-- 4 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/boot/bootmeth_extlinux.c b/boot/bootmeth_extlinux.c index aa2a4591eb..ae0ad1d53e 100644 --- a/boot/bootmeth_extlinux.c +++ b/boot/bootmeth_extlinux.c @@ -82,7 +82,7 @@ static int extlinux_fill_info(struct bootflow *bflow) log_debug("parsing bflow file size %x\n", bflow->size); membuff_init(&mb, bflow->buf, bflow->size); membuff_putraw(&mb, bflow->size, true, &data);
  while (len = membuff_readline(&mb, line, sizeof(line) - 1, ' '), len) {
  while (len = membuff_readline(&mb, line, sizeof(line) - 1, ' ', true), len) {
          char *tok, *p = line;

          tok = strsep(&p, " ");
diff --git a/common/console.c b/common/console.c index 8a869b137e..cd56035171 100644 --- a/common/console.c +++ b/common/console.c @@ -846,7 +846,7 @@ bool console_record_overflow(void) int console_record_readline(char *str, int maxlen) { return membuff_readline((struct membuff *)&gd->console_out, str,
                          maxlen, '\0');
                          maxlen, '\0', false);
}

int console_record_avail(void) diff --git a/include/membuff.h b/include/membuff.h index 21051b0c54..4eba626ce1 100644 --- a/include/membuff.h +++ b/include/membuff.h @@ -192,10 +192,11 @@ int membuff_free(struct membuff *mb);

@mb: membuff to adjust

@str: Place to put the line

@maxlen: Maximum line length (excluding terminator)

@must_fit: If true then str is empty if line doesn't fit

Return: number of bytes read (including terminator) if a line has been
   read, 0 if nothing was there
   read, 0 if nothing was there or line didn't fit when must_fit is set
*/
-int membuff_readline(struct membuff *mb, char *str, int maxlen, int minch); +int membuff_readline(struct membuff *mb, char *str, int maxlen, int minch, bool must_fit);

/**

membuff_extend_by() - expand a membuff

diff --git a/lib/membuff.c b/lib/membuff.c index 36dc43a523..964e504d5b 100644 --- a/lib/membuff.c +++ b/lib/membuff.c @@ -288,7 +288,7 @@ int membuff_free(struct membuff *mb) (mb->end - mb->start) - 1 - membuff_avail(mb); }

-int membuff_readline(struct membuff *mb, char *str, int maxlen, int minch) +int membuff_readline(struct membuff *mb, char *str, int maxlen, int minch, bool must_fit)

Shouldn't that be the normal case? I might have this wrong, but this looks like a bug fix to me.

...

{ int len; /* number of bytes read (!= string length) */ char *s, *end; @@ -310,7 +310,7 @@ int membuff_readline(struct membuff *mb, char *str, int maxlen, int minch) }
    /* couldn't get the whole string */
  if (!ok) {
  if (!ok && must_fit) {
          if (maxlen)
                  *orig = '\0';
          return 0;
-- 2.40.1

Heinrich Schuchardt

22 Nov 22 Nov

5:15 a.m.

New subject: [PATCH v3 4/5] lib: membuff: fix readline not returning line in case of overflow

On 11/21/23 19:40, Simon Glass wrote:

...

Hi Svyatoslav,

On Tue, 21 Nov 2023 at 11:35, Svyatoslav Ryhel clamor95@gmail.com wrote:

...
From: Ion Agorria ion@agorria.com

If line overflows readline it will not be returned, fix this behavior, make it optional and documented properly.

Signed-off-by: Ion Agorria ion@agorria.com Signed-off-by: Svyatoslav Ryhel clamor95@gmail.com Reviewed-by: Mattijs Korpershoek mkorpershoek@baylibre.com

boot/bootmeth_extlinux.c | 2 +- common/console.c | 2 +- include/membuff.h | 5 +++-- lib/membuff.c | 4 ++-- 4 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/boot/bootmeth_extlinux.c b/boot/bootmeth_extlinux.c index aa2a4591eb..ae0ad1d53e 100644 --- a/boot/bootmeth_extlinux.c +++ b/boot/bootmeth_extlinux.c @@ -82,7 +82,7 @@ static int extlinux_fill_info(struct bootflow *bflow) log_debug("parsing bflow file size %x\n", bflow->size); membuff_init(&mb, bflow->buf, bflow->size); membuff_putraw(&mb, bflow->size, true, &data);
  while (len = membuff_readline(&mb, line, sizeof(line) - 1, ' '), len) {
  while (len = membuff_readline(&mb, line, sizeof(line) - 1, ' ', true), len) {
           char *tok, *p = line;

           tok = strsep(&p, " ");
diff --git a/common/console.c b/common/console.c index 8a869b137e..cd56035171 100644 --- a/common/console.c +++ b/common/console.c @@ -846,7 +846,7 @@ bool console_record_overflow(void) int console_record_readline(char *str, int maxlen) { return membuff_readline((struct membuff *)&gd->console_out, str,
                          maxlen, '\0');
                          maxlen, '\0', false);
}

int console_record_avail(void)
diff --git a/include/membuff.h b/include/membuff.h index 21051b0c54..4eba626ce1 100644 --- a/include/membuff.h +++ b/include/membuff.h @@ -192,10 +192,11 @@ int membuff_free(struct membuff *mb);

@mb: membuff to adjust

@str: Place to put the line

@maxlen: Maximum line length (excluding terminator)

@must_fit: If true then str is empty if line doesn't fit

Return: number of bytes read (including terminator) if a line has been
   read, 0 if nothing was there
   read, 0 if nothing was there or line didn't fit when must_fit is set
*/
-int membuff_readline(struct membuff *mb, char *str, int maxlen, int minch); +int membuff_readline(struct membuff *mb, char *str, int maxlen, int minch, bool must_fit);

/**

membuff_extend_by() - expand a membuff

diff --git a/lib/membuff.c b/lib/membuff.c index 36dc43a523..964e504d5b 100644 --- a/lib/membuff.c +++ b/lib/membuff.c @@ -288,7 +288,7 @@ int membuff_free(struct membuff *mb) (mb->end - mb->start) - 1 - membuff_avail(mb); }

-int membuff_readline(struct membuff *mb, char *str, int maxlen, int minch) +int membuff_readline(struct membuff *mb, char *str, int maxlen, int minch, bool must_fit)
Shouldn't that be the normal case? I might have this wrong, but this looks like a bug fix to me.

For the extlinux.conf case assuming that the file has ended if a line is truncated seems to be the wrong thing to do.

Currently we return a value <= maxlen - 1 if no truncation has occurred and 0 if a truncation occurred.

I would prefer if the function would return maxlen in the case of truncation and the truncated string. This will allow us to detect truncation without mistaking it for the end of the buffer.

Best regards

Heinrich

...

...
{ int len; /* number of bytes read (!= string length) */ char *s, *end; @@ -310,7 +310,7 @@ int membuff_readline(struct membuff *mb, char *str, int maxlen, int minch) }
     /* couldn't get the whole string */
  if (!ok) {
  if (!ok && must_fit) {
           if (maxlen)
                   *orig = '\0';
           return 0;
-- 2.40.1

Simon Glass

30 Nov 30 Nov

4:03 a.m.

New subject: [PATCH v3 4/5] lib: membuff: fix readline not returning line in case of overflow

Hi,

On Tue, 21 Nov 2023 at 21:16, Heinrich Schuchardt xypron.glpk@gmx.de wrote:

...

On 11/21/23 19:40, Simon Glass wrote:

...
Hi Svyatoslav,

On Tue, 21 Nov 2023 at 11:35, Svyatoslav Ryhel clamor95@gmail.com wrote:

...
From: Ion Agorria ion@agorria.com

If line overflows readline it will not be returned, fix this behavior, make it optional and documented properly.

Signed-off-by: Ion Agorria ion@agorria.com Signed-off-by: Svyatoslav Ryhel clamor95@gmail.com Reviewed-by: Mattijs Korpershoek mkorpershoek@baylibre.com

boot/bootmeth_extlinux.c | 2 +- common/console.c | 2 +- include/membuff.h | 5 +++-- lib/membuff.c | 4 ++-- 4 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/boot/bootmeth_extlinux.c b/boot/bootmeth_extlinux.c index aa2a4591eb..ae0ad1d53e 100644 --- a/boot/bootmeth_extlinux.c +++ b/boot/bootmeth_extlinux.c @@ -82,7 +82,7 @@ static int extlinux_fill_info(struct bootflow *bflow) log_debug("parsing bflow file size %x\n", bflow->size); membuff_init(&mb, bflow->buf, bflow->size); membuff_putraw(&mb, bflow->size, true, &data);
  while (len = membuff_readline(&mb, line, sizeof(line) - 1, ' '), len) {
  while (len = membuff_readline(&mb, line, sizeof(line) - 1, ' ', true), len) {
           char *tok, *p = line;

           tok = strsep(&p, " ");
diff --git a/common/console.c b/common/console.c index 8a869b137e..cd56035171 100644 --- a/common/console.c +++ b/common/console.c @@ -846,7 +846,7 @@ bool console_record_overflow(void) int console_record_readline(char *str, int maxlen) { return membuff_readline((struct membuff *)&gd->console_out, str,
                          maxlen, '\0');
                          maxlen, '\0', false);
}

int console_record_avail(void)
diff --git a/include/membuff.h b/include/membuff.h index 21051b0c54..4eba626ce1 100644 --- a/include/membuff.h +++ b/include/membuff.h @@ -192,10 +192,11 @@ int membuff_free(struct membuff *mb);

@mb: membuff to adjust

@str: Place to put the line

@maxlen: Maximum line length (excluding terminator)

@must_fit: If true then str is empty if line doesn't fit

Return: number of bytes read (including terminator) if a line has been
   read, 0 if nothing was there
   read, 0 if nothing was there or line didn't fit when must_fit is set
*/
-int membuff_readline(struct membuff *mb, char *str, int maxlen, int minch); +int membuff_readline(struct membuff *mb, char *str, int maxlen, int minch, bool must_fit);

/**

membuff_extend_by() - expand a membuff

diff --git a/lib/membuff.c b/lib/membuff.c index 36dc43a523..964e504d5b 100644 --- a/lib/membuff.c +++ b/lib/membuff.c @@ -288,7 +288,7 @@ int membuff_free(struct membuff *mb) (mb->end - mb->start) - 1 - membuff_avail(mb); }

-int membuff_readline(struct membuff *mb, char *str, int maxlen, int minch) +int membuff_readline(struct membuff *mb, char *str, int maxlen, int minch, bool must_fit)
Shouldn't that be the normal case? I might have this wrong, but this looks like a bug fix to me.
For the extlinux.conf case assuming that the file has ended if a line is truncated seems to be the wrong thing to do.

Currently we return a value <= maxlen - 1 if no truncation has occurred and 0 if a truncation occurred.

I would prefer if the function would return maxlen in the case of truncation and the truncated string. This will allow us to detect truncation without mistaking it for the end of the buffer.

OK I have taken a harder look at this now.

Wouldn't that indicate that the string was exactly the maximum length?

Also, what happens to the chars after that on the same line?

What is actually going wrong that is leading to the need for this patch? How can it be correct to parse just some of the line and leave the rest for later?

I think this needs a harder look, but I don't have enough info to help here.

...

Best regards

Heinrich

...
...
{ int len; /* number of bytes read (!= string length) */ char *s, *end; @@ -310,7 +310,7 @@ int membuff_readline(struct membuff *mb, char *str, int maxlen, int minch) }
     /* couldn't get the whole string */
  if (!ok) {
  if (!ok && must_fit) {
           if (maxlen)
                   *orig = '\0';
           return 0;
-- 2.40.1

Regards, Simon

Svyatoslav Ryhel

21 Nov 21 Nov

7:35 p.m.

New subject: [PATCH v3 5/5] fastboot: add oem console command support

From: Ion Agorria ion@agorria.com

"oem console" serves to read console record buffer.

Signed-off-by: Ion Agorria ion@agorria.com Signed-off-by: Svyatoslav Ryhel clamor95@gmail.com Reviewed-by: Mattijs Korpershoek mkorpershoek@baylibre.com --- doc/android/fastboot.rst | 1 + drivers/fastboot/Kconfig | 7 +++++++ drivers/fastboot/fb_command.c | 39 +++++++++++++++++++++++++++++++++++ include/fastboot.h | 1 + 4 files changed, 48 insertions(+)

diff --git a/doc/android/fastboot.rst b/doc/android/fastboot.rst index 1ad8a897c8..05d8f77759 100644 --- a/doc/android/fastboot.rst +++ b/doc/android/fastboot.rst @@ -29,6 +29,7 @@ The following OEM commands are supported (if enabled): with <arg> = boot_ack boot_partition - ``oem bootbus`` - this executes ``mmc bootbus %x %s`` to configure eMMC - ``oem run`` - this executes an arbitrary U-Boot command +- ``oem console`` - this dumps U-Boot console record buffer

Support for both eMMC and NAND devices is included.

diff --git a/drivers/fastboot/Kconfig b/drivers/fastboot/Kconfig index 837c6f1180..58b08120a4 100644 --- a/drivers/fastboot/Kconfig +++ b/drivers/fastboot/Kconfig @@ -241,6 +241,13 @@ config FASTBOOT_OEM_RUN this feature if you are using verified boot, as it will allow an attacker to bypass any restrictions you have in place.

+config FASTBOOT_CMD_OEM_CONSOLE + bool "Enable the 'oem console' command" + depends on CONSOLE_RECORD + help + Add support for the "oem console" command to input and read console + record buffer. + endif # FASTBOOT

endmenu diff --git a/drivers/fastboot/fb_command.c b/drivers/fastboot/fb_command.c index 6f621df074..f95f4e4ae1 100644 --- a/drivers/fastboot/fb_command.c +++ b/drivers/fastboot/fb_command.c @@ -41,6 +41,7 @@ static void reboot_recovery(char *, char *); static void oem_format(char *, char *); static void oem_partconf(char *, char *); static void oem_bootbus(char *, char *); +static void oem_console(char *, char *); static void run_ucmd(char *, char *); static void run_acmd(char *, char *);

@@ -108,6 +109,10 @@ static const struct { .command = "oem run", .dispatch = CONFIG_IS_ENABLED(FASTBOOT_OEM_RUN, (run_ucmd), (NULL)) }, + [FASTBOOT_COMMAND_OEM_CONSOLE] = { + .command = "oem console", + .dispatch = CONFIG_IS_ENABLED(FASTBOOT_CMD_OEM_CONSOLE, (oem_console), (NULL)) + }, [FASTBOOT_COMMAND_UCMD] = { .command = "UCmd", .dispatch = CONFIG_IS_ENABLED(FASTBOOT_UUU_SUPPORT, (run_ucmd), (NULL)) @@ -159,6 +164,23 @@ void fastboot_multiresponse(int cmd, char *response) case FASTBOOT_COMMAND_GETVAR: fastboot_getvar_all(response); break; + case FASTBOOT_COMMAND_OEM_CONSOLE: + if (CONFIG_IS_ENABLED(FASTBOOT_CMD_OEM_CONSOLE)) { + char buf[FASTBOOT_RESPONSE_LEN] = { 0 }; + + if (console_record_isempty()) { + console_record_reset(); + fastboot_okay(NULL, response); + } else { + int ret = console_record_readline(buf, sizeof(buf) - 5); + + if (ret < 0) + fastboot_fail("Error reading console", response); + else + fastboot_response("INFO", response, "%s", buf); + } + break; + } default: fastboot_fail("Unknown multiresponse command", response); break; @@ -503,3 +525,20 @@ static void __maybe_unused oem_bootbus(char *cmd_parameter, char *response) else fastboot_okay(NULL, response); } + +/** + * oem_console() - Execute the OEM console command + * + * @cmd_parameter: Pointer to command parameter + * @response: Pointer to fastboot response buffer + */ +static void __maybe_unused oem_console(char *cmd_parameter, char *response) +{ + if (cmd_parameter) + console_in_puts(cmd_parameter); + + if (console_record_isempty()) + fastboot_fail("Empty console", response); + else + fastboot_response(FASTBOOT_MULTIRESPONSE_START, response, NULL); +} diff --git a/include/fastboot.h b/include/fastboot.h index 59cbea61ec..1e7920eb91 100644 --- a/include/fastboot.h +++ b/include/fastboot.h @@ -47,6 +47,7 @@ enum { FASTBOOT_COMMAND_OEM_PARTCONF, FASTBOOT_COMMAND_OEM_BOOTBUS, FASTBOOT_COMMAND_OEM_RUN, + FASTBOOT_COMMAND_OEM_CONSOLE, FASTBOOT_COMMAND_ACMD, FASTBOOT_COMMAND_UCMD, FASTBOOT_COMMAND_COUNT

-- 2.40.1

529

Age (days ago)

538

Last active (days ago)

List overview

Download

8 comments

3 participants

tags (0)

participants (3)

Heinrich Schuchardt
Simon Glass
Svyatoslav Ryhel