[openib-general] [PATCH] osm: 'chmod' on Windows

[Yevgeny Kliteynik]

Sasha Khapyorsky wrote:
> On 17:51 Sun 26 Nov     , Yevgeny Kliteynik wrote:
>> Sasha Khapyorsky wrote:
>>> On 15:22 Sun 26 Nov     , Yevgeny Kliteynik wrote:
>>>> Fixing to match 'chmod' implementation on Windows.
>>>> Note that on Windows 'chmod' there's no such thing 
>>>> as user/group/all permissions - the permissions are 
>>>> aways for 'all'.
>>>> Don't see why this would be a problem in this case,
>>>> but still - worth mentioning.
>>> It is because SA dump file contains privileged information like lists
>>> of subscribed ports.
>> Well, since it's being dumped in the same directory as the OSM log, 
>> I guess it gets the same protection as the OSM log does, doesn't it?
> 
> No, the file is readable by owner only.

IMHO, the directory where OSM log is written is readable only by owner (which is administrator).
If administrator choses to write osm log to some directory that is accessible to all users,
there's no way to remove read permissions from a certain *file* when doing fopen().
Same with SA dump file when doing chmod(), so as long as it is dumped in a protected directory,
it gets the same level of protection as the osm.log.
 
Anyway, what I'm saying is that I think that it's ok to use (_S_IREAD | _S_IWRITE) on windows 
instead of S_IRUSR | S_IWUSR) on Linux.
 
-- Yevgeny
> 
> Sasha
> 
>> -- Yevgeny
>>  
>>>> --
>>>> Yevgeny
>>>>
>>>> Signed-off-by:  Yevgeny Kliteynik <kliteyn at dev.mellanox.co.il>
>>>>  
>>>> Index: opensm/osm_sa.c
>>>> ===================================================================
>>>> --- opensm/osm_sa.c	(revision 10161)
>>>> +++ opensm/osm_sa.c	(working copy)
>>>> @@ -70,6 +70,9 @@
>>>>  #include <opensm/osm_multicast.h>
>>>>  #include <opensm/osm_inform.h>
>>>>  #include <opensm/osm_service.h>
>>>> +#ifdef WIN32
>>>> +#include <io.h>
>>>> +#endif /* WIN32 */
>>>>  
>>>>  #define  OSM_SA_INITIAL_TID_VALUE 0xabc
>>>>  
>>>> @@ -569,7 +572,11 @@ opensm_dump_to_file(osm_opensm_t *p_osm,
>>>>  		return -1;
>>>>  	}
>>>>  
>>>> +#ifndef WIN32
>>>>  	chmod(path, S_IRUSR|S_IWUSR);
>>>> +#else
>>>> +	_chmod(path, _S_IREAD|_S_IWRITE);
>>>> +#endif
>>> It would be very nice to not put such unneeded #ifdef(s) around common
>>> sources. You can simply wrap this function in your WIN32 specific
>>> config.h, something like:
>>>
>>> #define chmod(a,b) _chmod(a,b)
>>> #define S_IRUSR _S_IREAD
>>> #define S_IWUSR _S_IWRITE
>>>
>>> and this should solve this problem.
>>>
>>> Sasha
>>>
>