[PATCH] Re: [1.5] ls -l on /cygdrive/d doesn't work

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] Re: [1.5] ls -l on /cygdrive/d doesn't work

David Engraf-3


I have fixed the error in ntea.cc handling the return value of
NTQueryEaFile. This patch is only needed for the 1.5 release. Maybe this
error should be considered as critical due to uninitialized stack usage
of the variable fea when the function returned an error.


2009-05-26 David Engraf <[hidden email]>

        * ntea.cc (read_ea): Fix error handling and avoid using
        uninitialized stack.


David Engraf schrieb:

> I think this error is located in the cygwin/ntea.cc read_ea function.
> NtQueryEaFile fails due to unsupported extended attributes on fat32 and
> iso9660 and ret is set to -1. After setting ret to -1 the function
> checks fea->EaValueLength which is in my case 8313 (see log) due to an
> uninitialized stack and read_ea return 8313. Now the calling function
> (fhandler_base::fstat_helper) is using the uninitialized data for the
> timestamp and file size and returns incorrect values.
> This error doesn't happen on the new 1.7 release, but I need a solution
> for the stable version.
>
>
> if (!NT_SUCCESS (status))
>     {
>       ret = -1;
>       debug_printf ("%x = NtQueryEaFile (%s, %s), Win32 error %d",
>                     status, file, attrname, RtlNtStatusToDosError
> (status));
>     }
>   if (!fea->EaValueLength)
>     ret = 0;
>   else
>     {
>       memcpy (attrbuf, fea->EaName + fea->EaNameLength + 1,
>               fea->EaValueLength);
>       ret = fea->EaValueLength;
>     }
>
>
>
> David Engraf schrieb:
>> Hi,
>>
>> I have encountered a problem while listening the content of a CD. When
>> I call "ls -l /cygdrive/d" the file size and creation/modification
>> time is corrupted. This also happens on usb sticks formatted with
>> fat32. Only ntfs formatted filesystems have the correct listening.
>> Attached is a strace log calling the function fstat on a file located
>> on the specified filesystem.
>>
>> //ISO9660
>> get_file_attribute: file: d:\README.txt
>> read_ea: 1. chance, C0000010 = NtQueryEaFile (d:\README.txt,
>> .UNIXATTR), Win32 error 1
>> read_ea: C0000010 = NtQueryEaFile (d:\README.txt, .UNIXATTR), Win32
>> error 1
>> read_ea: 8313 = read_ea (0, d:\README.txt, .UNIXATTR, 22A9F0, 4)
>> fhandler_base::fstat_helper: 0 = fstat (, 0x22A9E0) st_atime=6B126FF
>> st_size=-40681930227712, st_mode=0x22A9F061,
>> st_ino=-4583408731929810241, sizeof=96
>> fstat64: 0 = fstat (3, 0x22A9E0)
>>
>>
>> //FAT32
>> get_file_attribute: file: e:\README.txt
>> read_ea: 1. chance, C000004F = NtQueryEaFile (e:\README.txt,
>> .UNIXATTR), Win32 error 282
>> read_ea: C000004F = NtQueryEaFile (e:\README.txt, .UNIXATTR), Win32
>> error 282
>> read_ea: 8313 = read_ea (0, e:\README.txt, .UNIXATTR, 22A9F0, 4)
>> fhandler_base::fstat_helper: 0 = fstat (, 0x22A9E0) st_atime=6D7072
>> st_size=3328772759537140781, st_mode=0x22A9F061,
>> st_ino=-3532118121688219773, sizeof=96
>> fstat64: 0 = fstat (3, 0x22A9E0)
>>
>>
>> //NTFS
>> get_file_attribute: file: c:\README.txt
>> read_ea: 0 = read_ea (6BC, c:\README.txt, .UNIXATTR, 22A410, 4)
>> fhandler_base::fstat_helper: 0 = fstat (, 0x22A400) st_atime=4A1A7CA3
>> st_size=3088, st_mode=0x8124, st_ino=3096224743855743, sizeof=96
>> fstat64: 0 = fstat (6, 0x22A400)
>>
>>
>> Thank you
>>
>
--
David Engraf
Product Engineer

SYSGO AG
Office Mainz
Am Pfaffenstein 14 / D-55270 Klein-Winternheim / Germany

Handelsregister: HRB Mainz 90 HRB 8066
Vorstand: Michael Tiedemann
Aufsichtsratsvorsitzender: Knut Degen
USt(VAT)-Id-Nr.: DE 149062328

--- cygwin-1.5.25-15/winsup/cygwin/ntea.cc_orig 2008-04-16 14:57:15.000000000 +0200
+++ cygwin-1.5.25-15/winsup/cygwin/ntea.cc 2009-05-26 12:05:39.000000000 +0200
@@ -85,13 +85,16 @@ read_ea (HANDLE hdl, const char *file, c
       debug_printf ("%x = NtQueryEaFile (%s, %s), Win32 error %d",
     status, file, attrname, RtlNtStatusToDosError (status));
     }
-  if (!fea->EaValueLength)
-    ret = 0;
   else
     {
-      memcpy (attrbuf, fea->EaName + fea->EaNameLength + 1,
-      fea->EaValueLength);
-      ret = fea->EaValueLength;
+      if (!fea->EaValueLength)
+        ret = 0;
+      else
+        {
+          memcpy (attrbuf, fea->EaName + fea->EaNameLength + 1,
+          fea->EaValueLength);
+          ret = fea->EaValueLength;
+        }
     }
 
 out:


--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Problem reports:       http://cygwin.com/problems.html
Documentation:         http://cygwin.com/docs.html
FAQ:                   http://cygwin.com/faq/
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Re: [1.5] ls -l on /cygdrive/d doesn't work

Christopher Faylor-8
On Tue, May 26, 2009 at 12:41:03PM +0200, David Engraf wrote:

>I have fixed the error in ntea.cc handling the return value of
>NTQueryEaFile. This patch is only needed for the 1.5 release. Maybe this
>error should be considered as critical due to uninitialized stack usage
>of the variable fea when the function returned an error.
>
>
>2009-05-26 David Engraf <[hidden email]>
>
> * ntea.cc (read_ea): Fix error handling and avoid using
> uninitialized stack.

Thanks for the patch but this will have to be a known limitation of
1.5.x.  We don't plan on making any new releases before 1.7 is rolled
out.

cgf
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Re: [1.5] ls -l on /cygdrive/d doesn't work

David Engraf-3


Christopher Faylor wrote:

> On Tue, May 26, 2009 at 12:41:03PM +0200, David Engraf wrote:
>> I have fixed the error in ntea.cc handling the return value of
>> NTQueryEaFile. This patch is only needed for the 1.5 release. Maybe this
>> error should be considered as critical due to uninitialized stack usage
>> of the variable fea when the function returned an error.
>>
>>
>> 2009-05-26 David Engraf <[hidden email]>
>>
>> * ntea.cc (read_ea): Fix error handling and avoid using
>> uninitialized stack.
>
> Thanks for the patch but this will have to be a known limitation of
> 1.5.x.  We don't plan on making any new releases before 1.7 is rolled
> out.
>
> cgf

But this could be a security problem and as long as we don't have
released the 1.7 we should continue fixing critical parts because most
of the users are using the stable version.

- David