linux-kernel - Re: [syzbot] UBSAN: shift-out-of-bounds in profile

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:   Fri, 16 Jul 2021 23:10:25 +0900
From:   Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>
To:     Pavel Skripkin <paskripkin@...il.com>
Cc:     linux-kernel@...r.kernel.org, rostedt@...dmis.org,
        syzkaller-bugs@...glegroups.com, tglx@...utronix.de,
        syzbot <syzbot+e68c89a9510c159d9684@...kaller.appspotmail.com>
Subject: Re: [syzbot] UBSAN: shift-out-of-bounds in profile_init

On 2021/07/16 21:24, Pavel Skripkin wrote:
> But this function can be called not only from sysfs and I can't
> understand will my patch break something or not. And, I think, error
> message is needed somewhere here to inform callers about wrong shift
> value.
> 
> 
> Thoughts?

Subsequent profiling_store() attempts will return -EEXIST if
profile_setup() once set prof_on to non-zero value. Therefore,
if you try to return -EINVAL when profile_setup() returns 0,
you need to make sure that prof_on is set to non-zero value
only if prof_shift is valid.

But, the userspace might not be aware of the value of MAX_PROF_SHIFT
because it is an architecture dependent value, and par might become negative
value because get_option() accepts negative value. Therefore, it might be
better to

+		par = clamp(par, 0, MAX_PROF_SHIFT - 1);

than

+		if (par < 0 || par >= MAX_PROF_SHIFT)
+			return 0;

.