OS/2 shared folders found error for DOS programs [Fixed in SVN]

[FelixG]

I found 2 errors causing shared folders on OS/2 to not be able to correctly display file names in DOS windows and old DOS programs.

It is easily verifiable with DIR in a DOS window. Files in 8.3 name format should be displayed.

File: VboxSFFind.cpp

Function: vboxSfOs2IsUtf16Name8dot3

Replace: if (off >= offMax)

by: if (off > offMax)

Replace: if (ch != '.')

by: if (ch == '.')

Thanks and regards.

[FelixG]

I use the same ticket to give a solution to the problem of shared folders not working in some WPS.

File: VboxSFFind.cpp

Function: vboxSfOs2ReadDirEntries

Add below: EaOp.fpFEAList = (PFEALIST)pbData;

/* Maximum FEALIST size: */
if (pDataBuf->fLongFilenames)
| | (pEntry->cucShortName == 0)

EaOp.fpFEAList->cbList = cbData - 2 - (pEntry->name.u16Length / sizeof(RTUTF16));

else

EaOp.fpFEAList->cbList = cbData - 2 - (pEntry->cucShortName);

File: VboxSF.cpp

Function: vboxSfOs2MakeEmptyEaListEx

Provisionally cancel:

/* if ((char *)memchr(pszNameBuf, '\0', cbName) != &pszNameBuf[cbName])

{

cbDstList = pbSrc - 1 - (uint8_t *)pEaOp->fpGEAList;
rc = KernCopyOut(poffError, &cbDstList, sizeof(pEaOp->oError));
if (rc == NO_ERROR)

rc = ERROR_INVALID_EA_NAME;

Log(("vboxSfOs2MakeEmptyEaList: ERROR_INVALID_EA_NAME\n"));
break;

} */

I do not know the reason why the above does not work, but I propound 2 possible causes:

a) The lists may be of type GEA2 and FEA2

typedef struct _GEA2 {

ULONG oNextEntryOffset; <-- would be missing
BYTE cbName;
CHAR szname [ ? ];

} GEA2;
typedef struct _FEA2 {

ULONG NextEntryOffset; <-- would be missing
BYTE fEA;
BYTE cbName;
USHORT cbValue;
CHAR szName [ ? ];
BYTE bValue [ ? ];

} FEA2;

b) GEA->cbName may include the final character '\ 0'. ​This seems to be in the FAT32.IFS controller.

I include the references to the awesome work they did on said driver:

FAT32/trunk/src: ​http://trac.netlabs.org/fat32/browser/trunk/src?rev=0 FAT32/trunk/src/ifs/ifsea.c: ​http://trac.netlabs.org/fat32/browser/trunk/src/ifs/ifsea.c FAT32/trunk/src/ifs/ifsfind.c: ​http://trac.netlabs.org/fat32/browser/trunk/src/ifs/ifsfind.c

Thanks and regards.

[lerdmann]

Since I have been working on EA support in FAT32.IFS: 1) no, on the IFS level,the FEA and GEA structures are used and NOT the FEA2 and GEA2 structures. The kernel takes care of converting between FEA and FEA2 and GEA and GEA2 respectively. 2) yes,returning an empty EA list means to return the requested EA names in the FEA structure (taking a straight copy from the GEA structure) but with a zero length value (that is,no value and cbValue=0).

[FelixG]

You're right, I've already found the problem :D

I put the solution:

File: VboxSFFind.cpp

Function: vboxSfOs2ReadDirEntries

Add below: EaOp.fpFEAList = (PFEALIST)pbData;

/* Maximum FEALIST size: */
if (pDataBuf->fLongFilenames) | | (pEntry->cucShortName == 0)

EaOp.fpFEAList->cbList = cbData - 2 - (pEntry->name.u16Length / sizeof(RTUTF16));

else

EaOp.fpFEAList->cbList = cbData - 2 - (pEntry->cucShortName);

File: VboxSF.cpp

Function: vboxSfOs2MakeEmptyEaListEx

Replace: if ((char *)memchr(pszNameBuf, '\0', cbName) != &pszNameBuf[cbName])

by: if ((char *)memchr(pszNameBuf, '\0', (cbName + 1)) != &pszNameBuf[cbName])

[lerdmann]

I can confirm that these changes (and the test version that FelixG had sent to me) fixes the WPS problems with the shared folders under OS/2.

[FelixG]

In the latest version the fix has already been introduced in vboxSfOs2IsUtf16Name8dot3 (VboxSFFind.cpp),but the fixes are missing in vboxSfOs2ReadDirEntries (VboxSFFind.cpp) and in vboxSfOs2MakeEmptyEaListEx (VboxSf.cpp) for shared folders to work 100%.

Could you make the changes? #comment:3

[bird]

The problem with vboxSfOs2ReadDirEntries is that it needs to supply a FEAList length to vboxSfOs2MakeEmptyEaListEx. Writing it to the buffer isn't a safe solution as it's a user buffer which doesn't need to be valid. There is no need to subtract an filename length estimate either, as we'll notice we're out of space later when writing the filename - just subtracting it could easily lead to underflow and trampling on user data.

I noticed that jfs would return ERROR_EAS_DIDNT_FIT if it fails to return a single entry to userland. Doing that would require a little more trickery to the code, and it's maybe best to do this where vboxSfOs2ReadDirEntries is called instead... Didn't implement it yet.

The off-by-one fix to vboxSfOs2MakeEmptyEaListEx is classic, as I'm used to 'cbName' including the zero terminator. Fixed that and renamed the variable to cchName, which is what I call name lengths that does not include the terminator.

[bird]

Also noticed we weren't passing the alternative 8.3 filenames down from Windows hosts, so DOS and Win-OS/2 programs couldn't see files and directories with long filenames even if they could access them using the 8.3 alias directly. Guess we never needed it elsewhere. Next 6.1.x release will include this fix.

[FelixG]

"vboxSfOs2ReadDirEntries" function in case of "FI_LVL_EAS_FROM_LIST":
EaOp.fpGEAList = (PGEALIST)KernSelToFlat((uintptr_t)EaOp.fpGEAList)
...
EaOp.fpFEAList = (PFEALIST)pbData

fpGEAList points to an already existing structure, with a value already defined in fpGEAList-> cbList.

fpFEAList points to the buffer where we dump the information. fpFEAList-> cbList is not defined!, no one has defined it!

In my modification, it is defined as the maximum value of the buffer minus 1 byte (name length) and minus x bytes (name + null terminator). This allows it to be used to the maximum buffer size, without risk of overflow.

If we don't initialize it, the "vboxSfOs2MakeEmptyEaListEx" function will fail in:

    rc = KernCopyIn(&cbFullEasLeft, &pEaOp->fpFEAList->cbList, sizeof(cbFullEasLeft));
if (rc == NO_ERROR
    && cbGetEasLeft  >= sizeof(pEaOp->fpGEAList->cbList)
    && cbFullEasLeft >= sizeof(pEaOp->fpFEAList->cbList))

and it won't get to write the file name.

Regarding memchr (pszNameBuf, '\0', cbName):

"memchr" will look for the character '\0' from pszNameBuf[0] to pszNameBuf[cbName-1]. Obviously you won't be able to find it, because that character '\0' is in pszNameBuf[cbName].

For that reason it should be changed to: memchr(pszNameBuf, '\0', (cbName + 1))

I implemented these modifications, and in Ecomstation it worked perfectly (and I suppose that other OS2 versions that use EA will happen the same).

Regarding long names, with these modifications I have not observed any problem either.

Finally comment, that I find it very ugly that the point is closed, without having waited for my answer!

[bird]

I closed the defect because I had already reproduced the issue and fixed it. It would probably be easier if we pushed out the changes so you could verify if you like to: r93071 & r93073

The comment you added would've been very helpful when you filed the "patches", instead I had to analyze the code and proposed changes myself to understand it.

[FelixG]

If you had asked, I would have given you all kinds of explanations!

When the new version of VirtualBox comes out I will check if the problems have disappeared in my virtual machines, and I will write it here.

[lerdmann]

I can confirm that with the guest additions of VirtualBox 6.1.32 (in particular: vboxfs.ifs) I can now finally browse a shared folder from OS/2 without the WPS hanging or the like. EAs now work ok.

[FelixG]

I also confirm that it works correctly.