linux-kernel - Re: [PATCH] media: dvb_ringbuffer: Return -EFAULT if copy fails

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]

Message-ID: <41a9c6a6-4b9a-4d84-9e32-09bf64c65b6c@kili.mountain>
Date:   Wed, 24 May 2023 07:47:15 +0300
From:   Dan Carpenter <dan.carpenter@...aro.org>
To:     Su Hui <suhui@...china.com>
Cc:     Mauro Carvalho Chehab <mchehab@...nel.org>,
        YongSu Yoo <yongsuyoo0215@...il.com>,
        linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
        kernel-janitors@...r.kernel.org
Subject: Re: [PATCH] media: dvb_ringbuffer: Return -EFAULT if copy fails

On Wed, May 24, 2023 at 07:23:45AM +0300, Dan Carpenter wrote:
> On Wed, May 24, 2023 at 09:27:33AM +0800, Su Hui wrote:
> > The copy_to/from_user() functions return the number of bytes remaining
> > to be copied, but we want to return -EFAULT to the user.
> > 

So basically these bugs are caused because people are used to functions
returning negative error codes and they write some form of:

	ret = copy_from_user();
	if (ret)
		return ret;

If you look at the code and you think, "They author thinks 'ret' is
negative" then probably it is a bug.  The common false positives are
in the core kernel where it does:

	return copy_from_user();

and the caller checks:

	if (function_one() || function_two() || function_three())
		return -EFAULT;

Those are done because it's a fast path and adding a lot of if
statements would slow things down.  Driver code tends not to do this
because normally drivers are not so performance sensitive and it's more
important to be readable.

So you have to look at the context a bit.

regards,
dan carpenter