Re: [ros-dev] [ros-diffs] [pschweitzer] 58960: [RTL] Finally reenable RtlAcquirePrivilege

[Jérôme Gardou]

Hey Pierre!

Nice patch series, that takes ReactOS nearer to being secure. A remark though :
>  /*
> * enable the SeSystemtimePrivilege privilege
> * Call SetLocalTime twice to ensure correct results
> */
> Ret = SetLocalTime(&SetupData->SystemTime) &&
>      SetLocalTime(&SetupData->SystemTime);

This is a bit confusing, and the comment doesn't really help. Why would calling the function twice ansure correct result ? Also, this is likely to be optimized away by the compiler, or at least produce a report in static code analysers.

I know this isn't pure chance. Any hint as to why you wrote it like this ?

Cheers
Jérôme

[Hermès BÉLUSCA - MAÏTO]

IMHO he just kept the original code : see
http://svn.reactos.org/svn/reactos/trunk/reactos/dll/win32/syssetup/wizard.c
?r1=58960&r2=58959&pathrev=58960
You can discover the real culprit here :
http://svn.reactos.org/svn/reactos?view=revision&revision=21249 (6 years
ago) :D

Cheers,
Hermès

-----Message d'origine-----
De : [e-mail] [mailto:[e-mail]] De la
part de Jérôme Gardou
Envoyé : lundi 6 mai 2013 17:49
À : [e-mail]
Objet : Re: [ros-dev] [ros-diffs] [pschweitzer] 58960: [RTL] Finally
reenable RtlAcquirePrivilege

Hey Pierre!

Nice patch series, that takes ReactOS nearer to being secure. A remark
though :
>  /*
> * enable the SeSystemtimePrivilege privilege
> * Call SetLocalTime twice to ensure correct results
> */
> Ret = SetLocalTime(&SetupData->SystemTime) &&
>      SetLocalTime(&SetupData->SystemTime);

This is a bit confusing, and the comment doesn't really help. Why would
calling the function twice ansure correct result ? Also, this is likely to
be optimized away by the compiler, or at least produce a report in static
code analysers.

I know this isn't pure chance. Any hint as to why you wrote it like this ?

Cheers
Jérôme

_______________________________________________
Ros-dev mailing list
[e-mail]
http://www.reactos.org/mailman/listinfo/ros-dev

[Jose Catena]

Hi all,

Only an update of my situation: I have been working full time for a company
for two years now, and I'll continue doing so for undetermined time (read
long time). I'm still very interested in ROS and I keep working on it some
of the very little spare time I currently have. I do it fully independently
as I don't have the time to discuss what I'm doing, nor I assume that you
are interested in any way in what I do (most probably not from what we
discussed before), but if I ever get something finished to contribute, of
course you will know.

Best wishes,
Jose Catena

[Sven Barth]

On 06.05.2013 17:48, Jérôme Gardou wrote:
> Hey Pierre!
>
> Nice patch series, that takes ReactOS nearer to being secure. A remark though :
> >   /*
> > * enable the SeSystemtimePrivilege privilege
> > * Call SetLocalTime twice to ensure correct results
> >  */
> > Ret = SetLocalTime(&SetupData->SystemTime) &&
> >      SetLocalTime(&SetupData->SystemTime);
>
> This is a bit confusing, and the comment doesn't really help. Why would calling the function twice ansure correct result ? Also, this is likely to be optimized away by the compiler, or at least produce a report in static code analysers.
>
> I know this isn't pure chance. Any hint as to why you wrote it like this ?

Even MSDN states that it might be necessary to call SetLocalTime twice: 
http://msdn.microsoft.com/en-us/library/windows/desktop/ms724936%28v=vs.85%2...

Regards,
Sven

[Jérôme Gardou]

Thanks for the clarification. For some reason, I missed that bit.

ReactOS Development List [e-mail] wrote on lun., mai 6th, 2013, 8:25 :
> On 06.05.2013 17:48, Jérôme Gardou wrote:
> > Hey Pierre!
> >
> > Nice patch series, that takes ReactOS nearer to being secure. A remark though :
> > >   /*
> > > * enable the SeSystemtimePrivilege privilege
> > > * Call SetLocalTime twice to ensure correct results
> > >  */
> > > Ret = SetLocalTime(&SetupData->SystemTime) &&
> > >      SetLocalTime(&SetupData->SystemTime);
> >
> > This is a bit confusing, and the comment doesn't really help. Why would calling the function twice ansure correct result ? Also, this is likely to be optimized away by the compiler, or at least produce a report in static code analysers.
> >
> > I know this isn't pure chance. Any hint as to why you wrote it like this ?
>
> Even MSDN states that it might be necessary to call SetLocalTime twice: 
> http://msdn.microsoft.com/en-us/library/windows/desktop/ms724936%28v=vs.85%2...
>
> Regards,
> Sven
>
> Ros-dev mailing list
> [e-mail]
> http://www.reactos.org/mailman/listinfo/ros-dev