[PATCH 0/2] smb: client: fix DFS handling of non-ASCII paths with non-utf8 charset

[Fredric Cover <fredric.cover.lkernel@gmail.com>]

Hi Paulo,

This series fixes two problems that show up when a DFS mount uses a
non-utf8 charset (iocharset=iso8859-1, or a kernel built with a
non-utf8 CONFIG_NLS_DEFAULT) and the DFS path has non-ASCII
characters.

Patch 1 fixes an out-of-bounds read in dfs_parse_target_referral().
path_consumed is a utf8 byte count (the DFS cache works in utf8), but
it is applied to full_path, which is in the mount charset.  When the
count exceeds strlen(full_path), the unsigned subtraction wraps and
prepath points past the terminating NUL.

Patch 2 fixes a double conversion in setup_dfs_ref().  The path is
converted to utf8, then converted again by dfs_cache_find(), so
"é" is sent to the server as c3 83 c2 a9 and the second-level
referral fails with -ENOENT.

Order matters.  Patch 2 lets these mounts get past the second-level
referral, which makes the over-read from patch 1 reachable.  Applied
in the other order, patch 2 would turn a clean -ENOENT into an
out-of-bounds read (a KASAN report on KASAN kernels).

Testing: Samba 4.23 DFS root with a link named "café", client on a
KASAN kernel, with iocharset=iso8859-1 and the raw 0xe9 byte in the
mount path.
  - On an unmodified tree the mount fails with -ENOENT.
  - With only the setup_dfs_ref() change applied: KASAN
    slab-out-of-bounds read in dfs_parse_target_referral()
    (strlen(full_path)=29, path_consumed=30).
  - With both patches: the mount succeeds, no KASAN report.
  - Regression run: iocharset=utf8 with "café" mounts as before.

Known limitation, not addressed here: when a non-ASCII link is
followed by a path component (e.g. //srv/dfs/café/sub), path_consumed
is still in utf8 bytes while full_path is in the mount charset, so the
prefix path is offset incorrectly.  I saw this as -ENOENT.  Patch 1
only prevents the out-of-bounds access.  Fixing it needs the
consumed length converted to mount-charset units, which changes the
dfs_parse_target_referral() interface, so I haven't attempted it.
Paulo, how would you like that handled?

I haven't added Fixes tags.  The strlen() - path_consumed arithmetic
was moved by commit abdb1742a312 ("cifs: get rid of mount options
string parsing") and appears to date back further, and the double
conversion in setup_dfs_ref() also predates 5433c629e8d4, which only
moved the call. I couldn't pin down where either became reachable, so
I'd appreciate it if you could add the right tags. I haven't added
stable tags either; let me know if you'd like them.

Fredric Cover (2):
  smb: client: fix out-of-bounds read in dfs_parse_target_referral()
  smb: client: fix double conversion of DFS path in setup_dfs_ref()

 fs/smb/client/dfs.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

-- 
2.53.0