lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date: Wed, 15 May 2024 00:49:31 +0100 (BST)
From: "Maciej W. Rozycki" <macro@...am.me.uk>
To: Joe Perches <joe@...ches.com>
cc: Thomas Bogendoerfer <tsbogend@...ha.franken.de>, 
    linux-mips@...r.kernel.org, LKML <linux-kernel@...r.kernel.org>
Subject: Re: sibyte: pointless if tests

On Sat, 24 Feb 2024, Joe Perches wrote:

> Here are a couple in sibyte:
> 
> Maybe this should be documented as:
> 
> 	"well, don't know what to do here"

 Of course just removing these useless conditionals isn't going to help 
here because if a console write does fail for some reason, then the index 
will move backwards regardless.  At least we now have some kind of a 
placeholder to bring someone's attention (such as yours).

> $ spatch --very-quiet -sp-file if_semi.cocci .
> diff -u -p ./arch/mips/sibyte/common/cfe_console.c /tmp/nothing/arch/mips/sibyte/common/cfe_console.c
> --- ./arch/mips/sibyte/common/cfe_console.c
> +++ /tmp/nothing/arch/mips/sibyte/common/cfe_console.c
> @@ -22,8 +22,6 @@ static void cfe_console_write(struct con
>  		if (str[i] == '\n') {
>  			do {
>  				written = cfe_write(cfe_cons_handle, &str[last], i-last);
> -				if (written < 0)
> -					;
>  				last += written;
>  			} while (last < i);
>  			while (cfe_write(cfe_cons_handle, "\r", 1) <= 0)

 The author indeed clearly was undecided as to what to do as the full last 
statement here is actually:

			while (cfe_write(cfe_cons_handle, "\r", 1) <= 0)
				;

potentially looping indefinitely.

 The CFE API clearly says:

Error codes:
      Code              Description
      CFE_ERR_INV_PARAM File handle is invalid
      others            Device may return device-specific error codes

and maybe CFE never actually fails for a console device write (I'd have to 
check the sources if there are any "others" for the console device and I'd 
assume the console file handle is always valid), but IMO the safe approach 
would be just to chicken out and silently return on a failure from any of 
these calls.  Also this code has been oddly written IMO and would benefit 
from some refactoring.  I'll see if I can queue a patch.

  Maciej

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ