Changeset 93071 in vbox for trunk/src/VBox/Additions/os2/VBoxSF

Timestamp:

Dec 24, 2021 12:12:04 AM (3 years ago)

Author:

vboxsync

Message:

os2/VBoxSF: Fixed off by one check in vboxSfOs2MakeEmptyEaListEx. Fixed problem with missing FEAList length when called from vboxSfOs2ReadDirEntries. ticketref:19453

Location:

trunk/src/VBox/Additions/os2/VBoxSF

Files:

• VBoxSF.cpp (modified) (8 diffs)
• VBoxSFFind.cpp (modified) (1 diff)
• VBoxSFInternal.h (modified) (1 diff)

trunk/src/VBox/Additions/os2/VBoxSF/VBoxSF.cpp

@@ -1964 +1964 @@
  * @param   pEaOp           Kernel copy of the EA request with flattened pointers.
  * @param   uLevel          The info level being queried.
+ * @param   cbFullEasLeft   The size of the full EA buffer, ~(ULONG)0 if it
+ *                          should be read in from pEaOp->fpFEAList->cbList.
  * @param   pcbWritten      Where to return the length of the resulting list.  Optional.
  * @param   poffError       User buffer address of EAOP.oError for reporting GEALIST issues.
  */
-APIRET vboxSfOs2MakeEmptyEaListEx(PEAOP pEaOp, ULONG uLevel, uint32_t *pcbWritten, ULONG *poffError)
+APIRET vboxSfOs2MakeEmptyEaListEx(PEAOP pEaOp, ULONG uLevel, ULONG cbFullEasLeft, uint32_t *pcbWritten, ULONG *poffError)
 {
     ULONG  cbDstList;
@@ -1990 +1992 @@
         ULONG cbGetEasLeft = 0;
         rc = KernCopyIn(&cbGetEasLeft, &pEaOp->fpGEAList->cbList, sizeof(pEaOp->fpGEAList->cbList));
-        ULONG cbFullEasLeft = 0;
-        if (rc == NO_ERROR)
+        if (rc == NO_ERROR && cbFullEasLeft == ~(ULONG)0)
             rc = KernCopyIn(&cbFullEasLeft, &pEaOp->fpFEAList->cbList, sizeof(cbFullEasLeft));
         if (   rc == NO_ERROR
@@ -2013 +2014 @@
                  * pbSrc: GEA: BYTE cbName; char szName[];
                  */
-                /* Get name length. */
-                uint8_t cbName = 0;
-                rc = KernCopyIn(&cbName, pbSrc, sizeof(cbName));
-                Log3(("vboxSfOs2MakeEmptyEaList: cbName=%#x rc=%u\n", cbName, rc));
+                /* Get name length (we call it cchName instead of cbName since
+                   it does not include the zero terminator). */
+                uint8_t cchName = 0;
+                rc = KernCopyIn(&cchName, pbSrc, sizeof(cchName));
+                Log3(("vboxSfOs2MakeEmptyEaList: cchName=%#x rc=%u\n", cchName, rc));
                 if (rc != NO_ERROR)
                     break;
                 pbSrc++;
                 cbGetEasLeft--;
-                if (cbName + 1U > cbGetEasLeft)
+                if (cchName + 1U > cbGetEasLeft)
                 {
                     cbDstList = pbSrc - 1 - (uint8_t *)pEaOp->fpGEAList;
@@ -2032 +2034 @@
 
                 /* Copy in name. */
-                rc = KernCopyIn(pszNameBuf, pbSrc, cbName + 1);
+                rc = KernCopyIn(pszNameBuf, pbSrc, cchName + 1);
                 if (rc != NO_ERROR)
                     break;
-                Log3(("vboxSfOs2MakeEmptyEaList: szName: %.*Rhxs\n", cbName + 1, pszNameBuf));
-                if ((char *)memchr(pszNameBuf, '\0', cbName) != &pszNameBuf[cbName])
+                Log3(("vboxSfOs2MakeEmptyEaList: szName: %.*Rhxs\n", cchName + 1, pszNameBuf));
+                if ((char *)memchr(pszNameBuf, '\0', cchName + 1) != &pszNameBuf[cchName])
                 {
                     cbDstList = pbSrc - 1 - (uint8_t *)pEaOp->fpGEAList;
@@ -2047 +2049 @@
 
                 /* Skip input. */
-                cbGetEasLeft -= cbName + 1;
-                pbSrc        += cbName + 1;
+                cbGetEasLeft -= cchName + 1;
+                pbSrc        += cchName + 1;
 
                 /*
@@ -2054 +2056 @@
                  * Note! We should technically skip duplicates here, but who cares...
                  */
-                if (cbName > 0)
+                if (cchName > 0)
                 {
                     FEA Result;
-                    if (sizeof(Result) + cbName + 1 > cbFullEasLeft)
+                    if (sizeof(Result) + cchName + 1 <= cbFullEasLeft)
+                        cbFullEasLeft -= sizeof(Result) + cchName + 1;
+                    else
                     {
-                        Log(("vboxSfOs2MakeEmptyEaList: ERROR_BUFFER_OVERFLOW (%#x vs %#x)\n", sizeof(Result) + cbName + 1, cbFullEasLeft));
+                        Log(("vboxSfOs2MakeEmptyEaList: ERROR_BUFFER_OVERFLOW (%#x vs %#x)\n", sizeof(Result) + cchName + 1, cbFullEasLeft));
                         rc = ERROR_BUFFER_OVERFLOW;
                         break;
                     }
-                    cbFullEasLeft -= sizeof(Result) + cbName + 1;
 
                     Result.fEA     = 0;
-                    Result.cbName  = cbName;
+                    Result.cbName  = cchName;
                     Result.cbValue = 0;
                     rc = KernCopyOut(pbDst, &Result, sizeof(Result));
@@ -2073 +2076 @@
                     pbDst += sizeof(Result);
 
-                    rc = KernCopyOut(pbDst, pszNameBuf, cbName + 1);
+                    rc = KernCopyOut(pbDst, pszNameBuf, cchName + 1);
                     if (rc != NO_ERROR)
                         break;
-                    pbDst += cbName + 1;
+                    pbDst += cchName + 1;
                 }
             } /* (while more GEAs) */
@@ -2135 +2138 @@
         Log2(("vboxSfOs2MakeEmptyEaList: #0b: %p %p\n", EaOp.fpGEAList, EaOp.fpFEAList));
 
-        rc = vboxSfOs2MakeEmptyEaListEx(&EaOp, uLevel, NULL, &pEaOp->oError);
+        rc = vboxSfOs2MakeEmptyEaListEx(&EaOp, uLevel, ~(ULONG)0, NULL, &pEaOp->oError);
     }
     return rc;

trunk/src/VBox/Additions/os2/VBoxSF/VBoxSFFind.cpp

@@ -344 +344 @@
                         pbDst   = pbToCopy;
 
+                        /* Output empty EA list.  We don't try anticipate filename output length here,
+                           instead we'll just handle that when we come to it below. */
+                        /** @todo If this overflows, JFS will return ERROR_EAS_DIDNT_FIT and just the
+                         * EA size here (i.e. as if FI_LVL_STANDARD_EASIZE or _64 was requested).
+                         * I think, however, that ERROR_EAS_DIDNT_FIT should only be considered if
+                         * this is the first entry we're returning and we'll have to stop after it. */
                         uint32_t cbWritten = 0;
                         EaOp.fpFEAList = (PFEALIST)pbData;
-                        rc = vboxSfOs2MakeEmptyEaListEx(&EaOp, uLevel, &cbWritten, &pEaOpUser->oError);
+                        rc = vboxSfOs2MakeEmptyEaListEx(&EaOp, uLevel, cbData, &cbWritten, &pEaOpUser->oError);
                         if (rc == NO_ERROR)
                         {

trunk/src/VBox/Additions/os2/VBoxSF/VBoxSFInternal.h

@@ -249 +249 @@
 APIRET      vboxSfOs2CheckEaOpForCreation(EAOP const *pEaOp);
 APIRET      vboxSfOs2MakeEmptyEaList(PEAOP pEaOp, ULONG uLevel);
-APIRET      vboxSfOs2MakeEmptyEaListEx(PEAOP pEaOp, ULONG uLevel, uint32_t *pcbWritten, ULONG *poffError);
+APIRET      vboxSfOs2MakeEmptyEaListEx(PEAOP pEaOp, ULONG uLevel, ULONG cbFullEasLeft, uint32_t *pcbWritten, ULONG *poffError);
 
 DECLASM(PVBOXSFVP)  Fsh32GetVolParams(USHORT hVbp, PVPFSI *ppVpFsi /*optional*/);