Skip to content

Commit ac95b67

Browse files
committed
libretro-common/vfs_smb: fix silent truncations in read/write/stat
Three low-priority but real correctness issues in vfs_implementation_smb.c. None are memory-safety bugs; all three are silent data or size corruption that would confuse callers on unusual inputs. vfs_smb: cap len in retro_vfs_file_read_smb smb2_read takes uint32_t count, but the wrapper passes a uint64_t len straight through. For len > UINT32_MAX the high bits are silently dropped and smb2_read issues a truncated read. The caller receives a short return count and, depending on the loop, may re-issue from the wrong file offset or treat the short read as EOF. Fix: cap len at UINT32_MAX before the call. The VFS read contract already allows short reads (all well-behaved callers loop), so the caller simply re-issues for the remaining bytes. vfs_smb: cap len in retro_vfs_file_write_smb Identical bug class on the write side: smb2_write takes uint32_t count. Same fix. vfs_smb: saturate retro_vfs_stat_smb size *size is int64_t but st.smb2_size is uint64_t. A naked cast for files > INT64_MAX (8 EiB) produces a negative value that callers may interpret as an error. Saturate to INT64_MAX. Practically unreachable -- no filesystem today holds 8 EiB files -- but the fix is two lines and keeps sign semantics consistent with the matching fix in retro_vfs_stat_impl (commit cf73f1a). VC6 compatibility: the file is only compiled when libsmb2 is linked, which in practice is a modern toolchain concern. For correctness anyway, the fix uses UINT32_MAX and INT64_MAX which are defined by the existing VC6 stdint shim at include/compat/msvc/stdint.h, and adds an explicit reliably available even if libsmb2's transitive include chain ever changes. Regression test: NONE. Exercising the SMB VFS layer requires a live SMB server and an authenticated mount, which is impractical for a self-contained sample that runs in CI. The existing nbio_test and config_file_test exercise the non-SMB VFS paths; a regression that broke those paths would still surface. The fixes themselves are small (two cap statements and a ternary) and reviewed directly.
1 parent 18e15ad commit ac95b67

1 file changed

Lines changed: 25 additions & 3 deletions

File tree

libretro-common/vfs/vfs_implementation_smb.c

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
*/
1515

1616
#include <stdio.h>
17+
#include <stdint.h> /* UINT32_MAX, INT64_MAX -- via compat shim on VC6 */
1718
#include <string.h>
1819
#include <errno.h>
1920
#include <time.h>
@@ -352,7 +353,15 @@ int64_t retro_vfs_file_read_smb(libretro_vfs_implementation_file *stream,
352353
if (!ctx)
353354
return -1;
354355

355-
ret = smb2_read(ctx, (struct smb2fh *)(intptr_t)stream->smb_fh, s, len);
356+
/* smb2_read takes uint32_t count; passing a uint64_t len that
357+
* exceeds UINT32_MAX was silently truncated pre-patch. Cap at
358+
* UINT32_MAX so the caller gets back the (partial) byte count
359+
* and knows to loop for more. This matches fread-style
360+
* short-read semantics already observed by VFS consumers. */
361+
if (len > (uint64_t)UINT32_MAX)
362+
len = UINT32_MAX;
363+
364+
ret = smb2_read(ctx, (struct smb2fh *)(intptr_t)stream->smb_fh, s, (uint32_t)len);
356365

357366
return ret;
358367
}
@@ -370,7 +379,13 @@ int64_t retro_vfs_file_write_smb(libretro_vfs_implementation_file *stream,
370379
if (!ctx)
371380
return -1;
372381

373-
ret = smb2_write(ctx, (struct smb2fh *)(intptr_t)stream->smb_fh, (void*)s, len);
382+
/* smb2_write takes uint32_t count; see retro_vfs_file_read_smb
383+
* for the rationale. Cap at UINT32_MAX and let the caller
384+
* re-issue for remaining bytes. */
385+
if (len > (uint64_t)UINT32_MAX)
386+
len = UINT32_MAX;
387+
388+
ret = smb2_write(ctx, (struct smb2fh *)(intptr_t)stream->smb_fh, (void*)s, (uint32_t)len);
374389

375390
return ret;
376391
}
@@ -571,8 +586,15 @@ int retro_vfs_stat_smb(const char *path, int64_t *size)
571586
if (smb2_stat(smb_context, rel_path, &st) < 0)
572587
return 0;
573588

589+
/* smb2_size is uint64_t; *size is int64_t. A naked cast on
590+
* files > INT64_MAX (8 EiB) would produce a negative value
591+
* that callers may interpret as a stat error. Saturate to
592+
* INT64_MAX -- unreachable in practice today, but the fix is
593+
* cheap and keeps sign semantics sane. */
574594
if (size)
575-
*size = (int64_t)st.smb2_size;
595+
*size = (st.smb2_size > (uint64_t)INT64_MAX)
596+
? INT64_MAX
597+
: (int64_t)st.smb2_size;
576598

577599
return RETRO_VFS_STAT_IS_VALID |
578600
(st.smb2_type == SMB2_TYPE_DIRECTORY ? RETRO_VFS_STAT_IS_DIRECTORY : 0);

0 commit comments

Comments
 (0)