PING: RFR: 8236489: Java heap file on daxfs should be more secure

[David Holmes]

Correction ...

On 6/01/2020 5:22 pm, David Holmes wrote:
> Hi Yasumasa,
> 
> On 6/01/2020 2:13 pm, Yasumasa Suenaga wrote:
>> Please review. I need one more reviewer to push.
>>
>>>>    JBS: https://bugs.openjdk.java.net/browse/JDK-8236489
>>>>    webrev: http://cr.openjdk.java.net/~ysuenaga/JDK-8236489/webrev.00/
> 
> So you are expecting that if you build on a newer linux and run on an 
> older pre-3.11 version then this code:
> 
>   194   fd = os::open(dir, O_TMPFILE | O_RDWR, S_IRUSR | S_IWUSR);
> 
> will set fd to -1. But I can't see any documented guarantee that will be 
> the case. It will depend how extensively open examines the flags 
> argument for unexpected values. If the flag is simply ignored then you 
> will successfully open thew directory - no?

Presumably no - per the below. The intent is that O_TMPFILE will be 
defined in such a way that its use will cause an error on older kernels.

> This also looks suspect:
> 
>    67 #ifndef O_TMPFILE
>    68 #ifdef __O_TMPFILE
>    69 #define O_TMPFILE __O_TMPFILE
>    70 #endif
>    71 #endif
> 
> when would we have __O_TMPFILE but not O_TMPFILE? 

This question remains.

> The definition on Linux itself is:
> 
> #ifndef __O_TMPFILE
> #define __O_TMPFILE     020000000
> #endif
> 
> /* a horrid kludge trying to make sure that this will fail on old 
> kernels */
> #define O_TMPFILE (__O_TMPFILE | O_DIRECTORY)

This was in asm-generic/fcntl.h but I think I should have been looking 
at bits/fcntl-linux.h which does the above in a cleaner way:

#ifndef __O_TMPFILE
# define __O_TMPFILE   (020000000 | __O_DIRECTORY)
#endif

David

> I think if we don't have O_TMPFILE that should be the end of it.
> 
> ( I also don't like contaminating os_posix.cpp with non-POSIX 
> functionality. :( )
> 
> Thanks,
> David
> -----
> 
>>
>> Yasumasa
>>
>>
>> On 2020/01/03 10:53, Yasumasa Suenaga wrote:
>>> PING: Could you review it?
>>>
>>>>    JBS: https://bugs.openjdk.java.net/browse/JDK-8236489
>>>>    webrev: http://cr.openjdk.java.net/~ysuenaga/JDK-8236489/webrev.00/
>>>
>>>
>>> Yasumasa
>>>
>>>
>>> On 2019/12/26 11:50, Yasumasa Suenaga wrote:
>>>> Hi all,
>>>>
>>>> I filed this to JBS. Could you review?
>>>>
>>>>    JBS: https://bugs.openjdk.java.net/browse/JDK-8236489
>>>>    webrev: http://cr.openjdk.java.net/~ysuenaga/JDK-8236489/webrev.00/
>>>>
>>>> It has passed all tests on submit repo. 
>>>> (mach5-one-ysuenaga-JDK-8236489-20191226-0145-7795073)
>>>>
>>>>
>>>> Thanks,
>>>>
>>>> Yasumasa
>>>>
>>>>
>>>> On 2019/12/19 14:58, Yasumasa Suenaga wrote:
>>>>> Hi all,
>>>>>
>>>>> HotSpot allocates Java heap on daxfs if we pass -XX:AllocateHeapAt.
>>>>> It performs open(2) and unlink(2) on daxfs, and it is used via 
>>>>> mmap'ed address.
>>>>>
>>>>> mmap(2) would be called with MAP_SHARED, and it is not atomically 
>>>>> between open(2) and unlink(2). If malicious user open Java heap 
>>>>> file before unlink(2), it might be exposed.
>>>>>
>>>>> So I think we can use open(2) with O_TMPFILE instead of mkstemp(3) 
>>>>> as below.
>>>>>
>>>>>    http://cr.openjdk.java.net/~ysuenaga/dax/
>>>>>
>>>>> O_TMPFILE would create inode on filesystem, and it cannot be 
>>>>> accessed from out-of-process.
>>>>> However it cannot be provided in older Linux kernel. So I keep 
>>>>> current code as fall back.
>>>>>
>>>>>    http://man7.org/linux/man-pages/man2/open.2.html
>>>>>
>>>>> What do you think about it? or someone is working for it?
>>>>> If it is ok, I will file it to JBS and will send review request.
>>>>>
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Yasumasa
>>>>>
>>>>>
>>>>> P.S.
>>>>>    I tried to use MAP_PRIVATE for it, but it was slower than 
>>>>> MAP_SHARED.