Re: [U-Boot] [PATCH v2 2/2] efi_loader: Fix possible starving in efi_cin_read_key

On 3/5/19 12:50 PM, matthias.bgg@kernel.org wrote:

From: Matthias Brugger mbrugger@suse.com

The function efi_cin_read_key can be affected by a loss of a character which will make u-boot to wait forever. Fix this by calling term_get_char instead.

Signed-off-by: Matthias Brugger mbrugger@suse.com

You can test EFI keyboard input with:

=> setenv efi_selftest extended text input => bootefi selftest

Function keys, arrow keys, PG UP, PG DN, etc. do not work with this patch in place.

---

Changes in v2: None

lib/efi_loader/efi_console.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/lib/efi_loader/efi_console.c b/lib/efi_loader/efi_console.c index 1133674faf..558aaed109 100644 --- a/lib/efi_loader/efi_console.c +++ b/lib/efi_loader/efi_console.c @@ -493,13 +493,14 @@ static int analyze_modifiers(struct efi_key_state *key_state) { int c, mod = 0, ret = 0;

• c = getc();

• if (!term_get_char(&c))

• goto out;
  
  

  if (c != ';') { ret = c; if (c == '~') goto out;

• c = getc();
  

• term_get_char(&c);
  

lib/efi_loader/efi_console.c:496:21: warning: passing argument 1 of ‘term_get_char’ from incompatible pointer type [-Wincompatible-pointer-types] if (!term_get_char(&c)) ^~ lib/efi_loader/efi_console.c:65:32: note: expected ‘char *’ but argument is of type ‘int *’ static int term_get_char(char *c)

The problem repeats throughout the patch.

Best thing is to define the argument of term_get_char() as s32 in the first patch of the series.

} for (;;) { switch (c) { @@ -508,7 +509,7 @@ static int analyze_modifiers(struct efi_key_state *key_state) mod += c - '0'; /* fall through */ case ';':

• 	c = getc();
  

• 	term_get_char(&c);
  break;
  

  default: goto out;

@@ -551,7 +552,9 @@ static efi_status_t efi_cin_read_key(struct efi_key_data *key) * Xterm Control Sequences * https://www.xfree86.org/4.8.0/ctlseqs.html */

• ch = getc();
  

• if (!term_get_char(&ch))
  

• 	return EFI_NOT_READY;
  

• switch (ch) { case cESC: /* ESC */ pressed_key.scan_code = 23;

@@ -561,12 +564,15 @@ static efi_status_t efi_cin_read_key(struct efi_key_data *key)

You missed a getc() in line 560.

Best regards

Heinrich

	/* consider modifiers */
	if (ch < 'P') {
		set_shift_mask(ch - '0', &key->key_state);

• 		ch = getc();
  

• 		if (!term_get_char(&ch))
  

• 			return EFI_NOT_READY;
  }
  pressed_key.scan_code = ch - 'P' + 11;
  break;
  

  case '[':

• 	ch = getc();
  

• 	if (!term_get_char(&ch))
  

• 		return EFI_NOT_READY;
  

• switch (ch) {
  case 'A'...'D': /* up, down right, left */
  	pressed_key.scan_code = ch - 'A' + 1;