linux-kernel - Re: [PATCH 4/7] staging: comedi: don't allow read() on async command set up for "write"

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, 30 Oct 2014 20:27:33 +0000
From:	Ian Abbott <abbotti@....co.uk>
To:	Hartley Sweeten <HartleyS@...ionengravers.com>,
	"driverdev-devel@...uxdriverproject.org" 
	<driverdev-devel@...uxdriverproject.org>
CC:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 4/7] staging: comedi: don't allow read() on async command
 set up for "write"

On 30/10/14 18:05, Hartley Sweeten wrote:
> On Thursday, October 30, 2014 5:42 AM, Ian Abbott wrote:
[snip]
>>   	add_wait_queue(&async->wait_head, &wait);
>>   	while (nbytes > 0 && !retval) {
>> @@ -2249,6 +2253,10 @@ static ssize_t comedi_read(struct file *file, char __user *buf, size_t nbytes,
>>   				retval = -EACCES;
>>   				break;
>>   			}
>> +			if (async->cmd.flags & CMDF_WRITE) {
>> +				retval = -EINVAL;
>> +				break;
>> +			}
>
> Is this second test really needed in the while() loop?
>
> For that matter, are the s->busy tests needed in the while() loop?

To answer your second question, some other thread using the same file 
object might have cancelled the asynchronous command, causing the 
current thread to see that the command is no longer active when it wakes up.

To answer your first question, that other thread might have managed to 
set up another asynchronous command in before we wake up, and it might 
have been set up as a "write" command (if the subdevice supports 
commands in both directions).  This doesn't detect the case when the 
other thread has managed to set up another "read" command, but since the 
current read() call hasn't read any data yet, we can just pretend we 
didn't know about the original command and read data from the new 
command instead.  (After all, the calling thread can't prove the read() 
started before the first command was cancelled, so we can just pretend 
it didn't.)

-- 
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti@....co.uk> )=-
-=(                          Web: http://www.mev.co.uk/  )=-
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/