linux-kernel - Re: [PATCH 1/2] riscv: allow case-insensitive ISA string parsing

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: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 27 Apr 2023 17:36:25 +0800
From:   Yangyu Chen <cyy@...self.name>
To:     conor.dooley@...rochip.com
Cc:     ajones@...tanamicro.com, aou@...s.berkeley.edu, conor@...nel.org,
        cyy@...self.name, i@...ithal.me, linux-kernel@...r.kernel.org,
        linux-riscv@...ts.infradead.org, palmer@...belt.com,
        paul.walmsley@...ive.com, soha@...u.info, twd2.me@...il.com
Subject: Re: [PATCH 1/2] riscv: allow case-insensitive ISA string parsing

Hi, Conor

I have a different opinion about whether the isa string length should be
checked.

On Thu, 27 Apr 2023 10:04:34 +0100, Conor Dooley wrote:
> Preventing the input of absolute rubbish is dt-validate's job & if the dtb
> itself has been corrupted somehow I suspect that we have bigger problems
> than checking for "rv" will solve.

> > also do a strlen(isa) >= 4 check first, though. of_property_read_string()
> > will succeed even when the string is "".

> I don't think that checking that there are at least 4 characters isn't
> even sufficient. Either we should confirm that this is a valid riscv,isa
> to run on (so rv##ima w/ ## matching the kernel) or not bother at all.

What will happen if we have a bootloader in the future which allows
overriding isa string in the DT or ACPI table, the memory corruption could
happen if we didn't check it first.

Although the kernel will not boot in this case, anything about the user
input string should be parse carefuly that you never know what the future
code will be but leave a checker here will remind someone who will change
the parse in the future to check the length carefully.

So I agree with drew, we should do check strlen before check the first
two characters.

On Thu, 27 Apr 2023 10:04:34 +0100, Conor Dooley wrote:
> It's a different issue though, and I'd be inclined to revisit it in the
> future when the ACPI stuff is in, along with perhaps the cleanup parts
> of Heiko's series too.

Agreed.

Thanks,
Yangyu Chen