svn commit: r291481 - head/sys/compat/linuxkpi/common/include/linux

[Hans Petter Selasky]

On 12/02/15 21:29, Mateusz Guzik wrote:
> On Mon, Nov 30, 2015 at 09:24:12AM +0000, Hans Petter Selasky wrote:
>> Author: hselasky
>> Date: Mon Nov 30 09:24:12 2015
>> New Revision: 291481
>> URL: https://svnweb.freebsd.org/changeset/base/291481
>>
>> Log:
>>    Add more functions and types to the LinuxKPI.
>>
>>    MFC after:	1 week
>>    Sponsored by:	Mellanox Technologies
>>
>> Modified:
>>    head/sys/compat/linuxkpi/common/include/linux/file.h
>>    head/sys/compat/linuxkpi/common/include/linux/workqueue.h
>>
>> Modified: head/sys/compat/linuxkpi/common/include/linux/file.h
>> ==============================================================================
>> --- head/sys/compat/linuxkpi/common/include/linux/file.h	Mon Nov 30 09:13:04 2015	(r291480)
>> +++ head/sys/compat/linuxkpi/common/include/linux/file.h	Mon Nov 30 09:24:12 2015	(r291481)
>> @@ -2,7 +2,7 @@
>>    * Copyright (c) 2010 Isilon Systems, Inc.
>>    * Copyright (c) 2010 iX Systems, Inc.
>>    * Copyright (c) 2010 Panasas, Inc.
>> - * Copyright (c) 2013 Mellanox Technologies, Ltd.
>> + * Copyright (c) 2013-2015 Mellanox Technologies, Ltd.
>>    * All rights reserved.
>>    *
>>    * Redistribution and use in source and binary forms, with or without
>> @@ -125,6 +125,21 @@ get_unused_fd(void)
>>   	return fd;
>>   }
>>
>> +static inline int
>> +get_unused_fd_flags(int flags)
>> +{
>> +	struct file *file;
>> +	int error;
>> +	int fd;
>> +
>> +	error = falloc(curthread, &file, &fd, flags);
>> +	if (error)
>> +		return -error;
>> +	/* drop the extra reference */
>> +	fdrop(file, curthread);
>> +	return fd;
>> +}
>> +
>
> This does not look right.
>
> AFAIR Linux drivers are not going to install fds into kernel threads. So
> this would be used for a userspace thread, but then it would completely
> insecure.
>
> Linux model is to reserve a slot in the fd table, obtain a 'file' object
> and install it as the last step.
>
> FreeBSD installs the file right away, but this means an extra reference
> has to be held in case something else using the table closes the fd.
>
> As such, this fdrop can lead to a use-after-free as the file can be
> freed from this poin.
>
> I'm afraid there is no way around patching improted consumers.
>

Hi Mateusz,

Thanks for your input. Yes, there is a potential race there, but no 
use-after-free from what I can see, because the LinuxKPI always retrieve 
the file pointer by the file number using "fget_unlocked()".

I'll look into if we can delay the fdrop() until after the fd_install().

--HPS