Re: [ros-dev] [ros-diffs] [hbelusca] 74493: [NTOS]: Capture the counted BaseDllName unicode string into a local NULL-terminated buffer before calling wcsrchr on it (actually I think it would be better to create & use a simi...

[Thomas Faber]

On 2017-05-07 20:15, hbelusca(a)svn.reactos.org wrote:
>   ---
> trunk/reactos/ntoskrnl/io/iomgr/driver.c	[iso-8859-1] (original)
>  +++ trunk/reactos/ntoskrnl/io/iomgr/driver.c	[iso-8859-1] Sun May  7 18:15:12 2017
>  @@ -884,7 +884,7 @@
>       PDEVICE_NODE DeviceNode;
>       PDRIVER_OBJECT DriverObject;
>       NTSTATUS Status;
>  -    PWCHAR FileNameWithoutPath;
>  +    PWCHAR Buffer, FileNameWithoutPath;
>       PWSTR FileExtension;
>       PUNICODE_STRING ModuleName = &BootLdrEntry->BaseDllName;
>       PLDR_DATA_TABLE_ENTRY LdrEntry;
>  @@ -898,13 +898,19 @@
>       IopDisplayLoadingMessage(ModuleName);
>       InbvIndicateProgress();
>
>  +    Buffer = ExAllocatePool(PagedPool, ModuleName->Length + sizeof(UNICODE_NULL));
>  +    ASSERT(Buffer); 
Please don't call ExAllocatePool.
And please check for allocation failure. An assert is not acceptable.

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

Hi!
The assert is fixed (r74495). For ExAllocatePool, it would be better to change them all at
once in this file.

Hermès

-----Message d'origine-----
De : Ros-dev [mailto:[e-mail]] De la part de Thomas Faber
Envoyé : dimanche 7 mai 2017 20:49
À : ros-dev(a)reactos.org
Objet : Re: [ros-dev] [ros-diffs] [hbelusca] 74493: [NTOS]: Capture the counted
BaseDllName unicode string into a local NULL-terminated buffer before calling wcsrchr on
it (actually I think it would be better to create & use a simi...

On 2017-05-07 20:15, hbelusca(a)svn.reactos.org wrote:
>   ---
> trunk/reactos/ntoskrnl/io/iomgr/driver.c	[iso-8859-1] (original)
>  +++ trunk/reactos/ntoskrnl/io/iomgr/driver.c	[iso-8859-1] Sun May  7 18:15:12 2017
>  @@ -884,7 +884,7 @@
>       PDEVICE_NODE DeviceNode;
>       PDRIVER_OBJECT DriverObject;
>       NTSTATUS Status;
>  -    PWCHAR FileNameWithoutPath;
>  +    PWCHAR Buffer, FileNameWithoutPath;
>       PWSTR FileExtension;
>       PUNICODE_STRING ModuleName = &BootLdrEntry->BaseDllName;
>       PLDR_DATA_TABLE_ENTRY LdrEntry;
>  @@ -898,13 +898,19 @@
>       IopDisplayLoadingMessage(ModuleName);
>       InbvIndicateProgress();
>
>  +    Buffer = ExAllocatePool(PagedPool, ModuleName->Length + sizeof(UNICODE_NULL));
>  +    ASSERT(Buffer); 
Please don't call ExAllocatePool.
And please check for allocation failure. An assert is not acceptable.

_______________________________________________
Ros-dev mailing list
Ros-dev(a)reactos.org
http://www.reactos.org/mailman/listinfo/ros-dev

[Thomas Faber]

There are 12 ExAllocatePoolWithTag calls in that file, and 2
ExAllocatePool calls, one of which you added. The consistency argument
is really not on your side.
But my point is that when adding new code, you should please follow the
"new" guideline (you know, the one that's going on 20 years old).


On 2017-05-08 04:11, Hermès BÉLUSCA-MAÏTO wrote:
>   Hi!
>  The assert is fixed (r74495). For ExAllocatePool, it would be better to change them all
> at once in this file.
>
>  Hermès
>
>  -----Message d'origine-----
>  De : Ros-dev [mailto:[e-mail]] De la part de Thomas Faber
>  Envoyé : dimanche 7 mai 2017 20:49
>  À : ros-dev(a)reactos.org
>  Objet : Re: [ros-dev] [ros-diffs] [hbelusca] 74493: [NTOS]: Capture the counted
> BaseDllName unicode string into a local NULL-terminated buffer before calling wcsrchr on
> it (actually I think it would be better to create & use a simi...
>
>  On 2017-05-07 20:15, hbelusca(a)svn.reactos.org wrote:
> >   ---
> > trunk/reactos/ntoskrnl/io/iomgr/driver.c	[iso-8859-1] (original)
> >  +++ trunk/reactos/ntoskrnl/io/iomgr/driver.c	[iso-8859-1] Sun May  7 18:15:12 2017
> >  @@ -884,7 +884,7 @@
> >       PDEVICE_NODE DeviceNode;
> >       PDRIVER_OBJECT DriverObject;
> >       NTSTATUS Status;
> >  -    PWCHAR FileNameWithoutPath;
> >  +    PWCHAR Buffer, FileNameWithoutPath;
> >       PWSTR FileExtension;
> >       PUNICODE_STRING ModuleName = &BootLdrEntry->BaseDllName;
> >       PLDR_DATA_TABLE_ENTRY LdrEntry;
> >  @@ -898,13 +898,19 @@
> >       IopDisplayLoadingMessage(ModuleName);
> >       InbvIndicateProgress();
> >
> >  +    Buffer = ExAllocatePool(PagedPool, ModuleName->Length + sizeof(UNICODE_NULL));
> >  +    ASSERT(Buffer); 
>  Please don't call ExAllocatePool.
>  And please check for allocation failure. An assert is not acceptable.
>
>  _______________________________________________
>  Ros-dev mailing list
>  Ros-dev(a)reactos.org
>  http://www.reactos.org/mailman/listinfo/ros-dev
>
>
>  _______________________________________________
>  Ros-dev mailing list
>  Ros-dev(a)reactos.org
>  http://www.reactos.org/mailman/listinfo/ros-dev
>