Re: [ros-dev] [ros-diffs] [pschweitzer] 65166: [NTFS] Simplify buffer size management in GetNtfsFileRecord(). As tests shown, Windows doesn't care about the real size of the file record and expects the caller to pass the co...

[Thomas Faber]

On 2014-11-01 11:02, pschweitzer(a)svn.reactos.org wrote:
>   -    OutputBuffer->FileRecordLength =
> FileRecord->BytesInUse;
>  -    RtlCopyMemory(OutputBuffer->FileRecordBuffer, FileRecord,
> FileRecord->BytesInUse);
>  +    OutputBuffer->FileRecordLength = DeviceExt->NtfsInfo.BytesPerFileRecord;
>  +    RtlCopyMemory(OutputBuffer->FileRecordBuffer, FileRecord,
> DeviceExt->NtfsInfo.BytesPerFileRecord); 
Wait, now there's no check against OutputBufferLength at all? It should
at least be
min(DeviceExt->NtfsInfo.BytesPerFileRecord,
    Stack->Parameters.FileSystemControl.OutputBufferLength)
in the memcpy size. Or am I missing something?

[Pierre Schweitzer]

You're missing line 611 where we check against size:
Stack->Parameters.FileSystemControl.OutputBufferLength <
(FIELD_OFFSET(NTFS_FILE_RECORD_OUTPUT_BUFFER, FileRecordBuffer) +
DeviceExt->NtfsInfo.BytesPerFileRecord)

On 01/11/2014 11:13, Thomas Faber wrote:
>   On 2014-11-01 11:02, pschweitzer(a)svn.reactos.org
> wrote:
> >   -    OutputBuffer->FileRecordLength =
> > FileRecord->BytesInUse;
> >  -    RtlCopyMemory(OutputBuffer->FileRecordBuffer, FileRecord,
> > FileRecord->BytesInUse);
> >  +    OutputBuffer->FileRecordLength = DeviceExt->NtfsInfo.BytesPerFileRecord;
> >  +    RtlCopyMemory(OutputBuffer->FileRecordBuffer, FileRecord,
> > DeviceExt->NtfsInfo.BytesPerFileRecord); 
>  
>  Wait, now there's no check against OutputBufferLength at all? It should
>  at least be
>  min(DeviceExt->NtfsInfo.BytesPerFileRecord,
>      Stack->Parameters.FileSystemControl.OutputBufferLength)
>  in the memcpy size. Or am I missing something?
>  
>  _______________________________________________
>  Ros-dev mailing list
>  Ros-dev(a)reactos.org
>  http://www.reactos.org/mailman/listinfo/ros-dev
>   

-- 
Pierre Schweitzer <pierre at reactos.org>
System & Network Administrator
Senior Kernel Developer
ReactOS Deutschland e.V.

[Thomas Faber]

Ah, indeed. Looks good, thanks.

On 2014-11-01 11:14, Pierre Schweitzer wrote:
>   You're missing line 611 where we check against
> size:
>  Stack->Parameters.FileSystemControl.OutputBufferLength <
>  (FIELD_OFFSET(NTFS_FILE_RECORD_OUTPUT_BUFFER, FileRecordBuffer) +
>  DeviceExt->NtfsInfo.BytesPerFileRecord)
>  
>  On 01/11/2014 11:13, Thomas Faber wrote:
> > On 2014-11-01 11:02, pschweitzer(a)svn.reactos.org wrote:
> >> -    OutputBuffer->FileRecordLength = FileRecord->BytesInUse;
> >> -    RtlCopyMemory(OutputBuffer->FileRecordBuffer, FileRecord,
> FileRecord->BytesInUse);
> >> +    OutputBuffer->FileRecordLength =
> DeviceExt->NtfsInfo.BytesPerFileRecord;
> >> +    RtlCopyMemory(OutputBuffer->FileRecordBuffer, FileRecord,
> DeviceExt->NtfsInfo.BytesPerFileRecord);
> >
> > Wait, now there's no check against OutputBufferLength at all? It should
> > at least be
> > min(DeviceExt->NtfsInfo.BytesPerFileRecord,
> >     Stack->Parameters.FileSystemControl.OutputBufferLength)
> > in the memcpy size. Or am I missing something?

[Thomas Faber]

On 2014-11-01 11:13, Thomas Faber wrote:
>   On 2014-11-01 11:02, pschweitzer(a)svn.reactos.org
> wrote:
> >   -    OutputBuffer->FileRecordLength =
> > FileRecord->BytesInUse;
> >  -    RtlCopyMemory(OutputBuffer->FileRecordBuffer, FileRecord,
> > FileRecord->BytesInUse);
> >  +    OutputBuffer->FileRecordLength = DeviceExt->NtfsInfo.BytesPerFileRecord;
> >  +    RtlCopyMemory(OutputBuffer->FileRecordBuffer, FileRecord,
> > DeviceExt->NtfsInfo.BytesPerFileRecord); 
>  
>  Wait, now there's no check against OutputBufferLength at all? It should
>  at least be
>  min(DeviceExt->NtfsInfo.BytesPerFileRecord,
>      Stack->Parameters.FileSystemControl.OutputBufferLength)
>  in the memcpy size. Or am I missing something? 
Er... Stack->Parameters.FileSystemControl.OutputBufferLength -
FIELD_OFFSET(...) obviously