Bug 203 - some multibyte filenames cause smbd crash
some multibyte filenames cause smbd crash
Status: CLOSED FIXED
Product: Samba 3.0
Classification: Unclassified
Component: File Services
3.0.10
x86 FreeBSD
: P2 major
: none
Assigned To: Tim Potter
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2003-07-03 11:28 UTC by TAKAHASHI Motonobu
Modified: 2005-08-24 10:19 UTC (History)
3 users (show)

See Also:


Attachments
Patch to handle invalid multibyte strings gracefully (875 bytes, patch)
2003-07-10 18:16 UTC, Tim Potter
no flags Details
Don't let invalid byte sequences invalidate the conversion (971 bytes, patch)
2003-07-12 13:49 UTC, Steve Langasek
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description TAKAHASHI Motonobu 2003-07-03 11:28:19 UTC
If you create a file, the 9th byte of whose filename
(excluded the drive name and first \, for example if
you have a full path like C:\tmp\1234A, then the 9th
char is 'A') is the first byte of multibyte char, then
smbd aborts with smb_panic().

Here is the patch for this, 
  created by MORIYAMA, Masayuki <msyk@mtg.biglobe.ne.jp>

--- cut here ---
--- smbd/open.c.orig    2003-06-08 02:57:39.000000000 +0900
+++ smbd/open.c 2003-07-03 20:01:37.000000000 +0900
@@ -72,7 +72,7 @@
 static void check_for_pipe(char *fname)
 {
        /* special case of pipe opens */
-       char s[10];
+       pstring s;
        StrnCpy(s,fname,sizeof(s)-1);
        strlower(s);
        if (strstr(s,"pipe/")) {
--- cut here ---

P.S.
We can't choose samba-3.0.0beta2 at a bug report now!
Comment 1 Tim Potter 2003-07-06 20:23:05 UTC
Monyo, 3.0.0beta2 has been added as a version string since you submitted your
bug report.
Comment 2 Tim Potter 2003-07-06 20:42:27 UTC
Since you mention multibyte filenames in the bug summary, can you expand on what
your configuration is here?  I don't see any crashes with no character set
changes to smb.conf

A stack backtrace would also be handy if you have it.
Comment 3 TAKAHASHI Motonobu 2003-07-07 09:29:50 UTC
>your configuration is here?
>I don't see any crashes with no character set
>changes to smb.conf

Ok, my smb.conf is:

[global]
  unix charset = CP932
[homes]
  writeable = yes

Operation:

net use x: \\<samba>\<share>
x:
mkdir tmp; cd tmp
copy con 1234<a><a>.TXT
test
^Z

'<a>' is a 2byte character for example U+3044 (0x82A2 in CP932).


[2003/07/07 15:53:31, 5] smbd/filename.c:unix_convert(323)
  New file 1234‚¢‚¢.TXT
[2003/07/07 15:53:31, 3] smbd/dosmode.c:unix_mode(110)
  unix_mode(tmp/1234‚¢‚¢.TXT) returning 0744
[2003/07/07 15:53:31, 5] smbd/files.c:file_new(122)
  allocated file structure 9772, fnum = 13868 (1 used)
[2003/07/07 15:53:31, 10] smbd/open.c:open_file_shared1(832)
  open_file_shared: fname = tmp/1234‚¢‚¢.TXT, share_mode = 8040, ofun = 1, mode
= 744, oplock request = 3
[2003/07/07 15:53:31, 8] lib/util.c:is_in_path(1535)
  is_in_path: tmp/1234‚¢‚¢.TXT
[2003/07/07 15:53:31, 8] lib/util.c:is_in_path(1539)
  is_in_path: no name list.
[2003/07/07 15:53:31, 3] lib/util.c:unix_clean_name(580)
  unix_clean_name [tmp/1234‚¢‚¢.TXT]
[2003/07/07 15:53:31, 4] smbd/open.c:open_file_shared1(998)
  calling open_file with flags=0x0 flags2=0x0 mode=0744
[2003/07/07 15:53:31, 10] smbd/open.c:fd_open(51)
  fd_open: name tmp/1234‚¢‚¢.TXT, flags = 00 mode = 0744, fd = -1. No such file
or directory
[2003/07/07 15:53:31, 3] smbd/open.c:open_file(173)
  Error opening file tmp/1234‚¢‚¢.TXT (No such file or directory) (local_flags=0
) (flags=0)
[2003/07/07 15:53:31, 0] lib/charcnv.c:convert_string_allocate(273)
  Conversion error: Illegal multibyte sequence(<82>)
[2003/07/07 15:53:31, 0] lib/util.c:smb_panic(1462)
  PANIC: failed to create UCS2 buffer
[2003/07/07 15:53:31, 0] lib/util.c:smb_panic(1469)
  BACKTRACE: 14 stack frames:
   #0 bin/smbd(smb_panic+0x16b) [0x816d4a3]
   #1 bin/smbd(unix_strlower+0x38) [0x815ce8c]
   #2 bin/smbd(strlower_m+0x53) [0x81677b3]
   #3 bin/smbd [0x80a2d45]
   #4 bin/smbd [0x80a2f80]
   #5 bin/smbd(open_file_shared1+0x683) [0x80a4503]
   #6 bin/smbd(reply_ntcreate_and_X+0x62c) [0x808b170]
   #7 bin/smbd [0x80b09ae]
   #8 bin/smbd [0x80b0a4e]
   #9 bin/smbd(process_smb+0x187) [0x80b0cf3]
   #10 bin/smbd(smbd_process+0x1b2) [0x80b1662]
   #11 bin/smbd(main+0x6cb) [0x81b536f]
   #12 /lib/libc.so.6(__libc_start_main+0xbb) [0x4016114f]
   #13 bin/smbd(yp_get_default_domain+0x69) [0x8070de1]
Comment 4 Tim Potter 2003-07-10 00:55:45 UTC
This looks very similar to bug 203.
Comment 5 Tim Potter 2003-07-10 00:56:12 UTC
D'oh!  That should be bug 202.
Comment 6 Tim Potter 2003-07-10 18:11:17 UTC
I think this is a side-effect of the fix for your previous bug report:

http://lists.samba.org/pipermail/samba-technical/2003-June/045514.html

Instead of panicing when an illegal multibyte sequence is encountered, we are
returning -1.  Perhaps we should be returning the unconverted data.

Monyo, can you test out my patch?
Comment 7 Tim Potter 2003-07-10 18:16:05 UTC
Created attachment 47 [details]
Patch to handle invalid multibyte strings gracefully

Vorlon has pointed out that returning the input string may have some strange
side-effects, especially if the caller is expecting the string to have been
upper or lower cased.

I'm not 100% convinced we need to go crazy and try and upper case each
individual character in the string.  If we have received an invalid sequence of
characters then something has gone wrong and I don't think we should try too
hard to fix it.
Comment 8 TAKAHASHI Motonobu 2003-07-12 09:33:39 UTC
>Monyo, can you test out my patch?

Sorry for too late reply,
Applying your patch (id=47), the problem is solved.




Comment 9 Steve Langasek 2003-07-12 13:42:52 UTC
My main worry is that other callers of convert_string_allocate() are expecting
conversions between *very* different charsets (UCS2->UTF8, or vice versa), and
returning an unconverted string may simply move this bug's manifestation
somewhere else (somewhere not bounded by an explicit assert()).

I've worked up a patch which, according to the /published/ behavior of iconv()
:), should let convert_string_allocate() skip over any invalid input sequences.
 I'll send it up here shortly.
Comment 10 Steve Langasek 2003-07-12 13:49:16 UTC
Created attachment 48 [details]
Don't let invalid byte sequences invalidate the conversion

This patch should ensure that invalid sequences don't prevent us from returning
a string in the requested charset: the invalid bytes will simply be skipped
over.  This should make callers of convert_string_allocate() a bit happier in
cases of charset misconfiguration.

I haven't been able test this patch yet -- unfortunately, I don't seem to have
the right combination of clients to trigger this bug, no matter how badly I
misconfigure my smb.conf.
Comment 11 TAKAHASHI Motonobu 2003-07-13 10:00:45 UTC
>My main worry is that other callers of convert_string_allocate() are expecting
conversions between *very* different charsets (UCS2->UTF8, or vice versa), and
returning an unconverted string may simply move this bug's manifestation
somewhere else (somewhere not bounded by an explicit assert()).

To fix the problem completely, I think the caller should receive the result 
(EINVAL, EILSEQ) and should determine what to do.
For example, this function is called in the filename matching, so easily to 
skip illegal bytes may cause funny file name matching, I think.

Comment 12 Tim Potter 2003-07-13 17:22:26 UTC
Monyo, I agree that making the callers check the return value is the correct
thing to do from a software engineering perspective.  Unfortunately I don't
think that would be practical as the callers of convert_string() are numerous
and callers of those functions number in the hundreds, if not thousands.  )-:

For example all the parsing of strings from RPC packets are done using
rpcstr_pull().  I don't think it's possible to go through every usage of this
function and check the return value is not -1.  Most of the time the return
value isn't even used and besides, what would we do if it did?  We're back at
the same position we are now: either panic or continue with known bad data.
Comment 13 Andrew Bartlett 2003-07-20 04:15:37 UTC
I have worked on part of this problem - the StrCaseCmp() calls to
unix_strupper().  The patch in current CVS should fix *some* of these segfaults.

(It works on the basis that an invalid string cannot be case-equal to any other
string, execpt a byte-for-byte representation)


Comment 14 Joachim Haga 2003-08-28 14:35:51 UTC
This may already be solved, just thought I'd pipe in with my own stacktrace
since it's a bit different from the one already posted. Just disregard if it's
not interesting. Beta2 on debian.

[2003/08/26 20:23:34, 0] lib/iconv.c:utf8_push(515)
  short utf8 write
[2003/08/26 20:23:34, 0] lib/charcnv.c:convert_string(194)
  convert_string: Required 144, available 72
...
[2003/08/26 20:23:35, 0] lib/iconv.c:utf8_pull(471)
  short utf8 char
[2003/08/26 20:23:35, 0] lib/charcnv.c:convert_string_allocate(272)
  Conversion error: Incomplete multibyte sequence(é)
[2003/08/26 20:23:35, 0] lib/util.c:smb_panic(1452)
  smb_panic(): calling panic action [/usr/share/samba/panic-action 22719]
[2003/08/26 20:23:36, 0] lib/util.c:smb_panic(1460)
  smb_panic(): action returned status 0
[2003/08/26 20:23:36, 0] lib/util.c:smb_panic(1462)
  PANIC: failed to create UCS2 buffer
[2003/08/26 20:23:36, 0] lib/util.c:smb_panic(1469)
  BACKTRACE: 16 stack frames:
   #0 /usr/sbin/smbd(smb_panic+0xc9) [0x817eb3d]
   #1 /usr/sbin/smbd(unix_strupper+0x89) [0x816e7dd]
   #2 /usr/sbin/smbd(strupper_m+0x44) [0x8178cf4]
   #3 /usr/sbin/smbd [0x80bd07e]
   #4 /usr/sbin/smbd [0x80bd6eb]
   #5 /usr/sbin/smbd(mangle_map+0x66) [0x80bbd5e]
   #6 /usr/sbin/smbd [0x809fac1]
   #7 /usr/sbin/smbd [0x80a0f69]
   #8 /usr/sbin/smbd(reply_trans2+0x52f) [0x80a625b]
   #9 /usr/sbin/smbd [0x80b670d]
   #10 /usr/sbin/smbd [0x80b6903]
   #11 /usr/sbin/smbd(process_smb+0x74) [0x80b6ab0]
   #12 /usr/sbin/smbd(smbd_process+0x198) [0x80b7578]
   #13 /usr/sbin/smbd(main+0x440) [0x81d5140]
   #14 /lib/libc.so.6(__libc_start_main+0xdd) [0x401c2a51]
   #15 /usr/sbin/smbd(chroot+0x31) [0x8075fa5]
Comment 15 Tim Potter 2003-08-28 16:49:10 UTC
Joachim, I don't suppose you could run the same test on the soon to be released
rc2 version of Samba?
Comment 16 Gerald (Jerry) Carter 2003-08-29 08:22:00 UTC
Looks like this should be fixed in RC2.
Comment 17 Joachim Haga 2003-08-30 07:15:15 UTC
RC2 doesn't crash, excellent!

Nautilus now complains about "illegal unicode" when it views the directory in
question. It may be a problem on the client side (linux smbfs), don't know. I
can't try it from windows at the moment.
Comment 18 TAKAHASHI Motonobu 2005-02-02 08:30:27 UTC
This bug still remains in Samba 3.0.10.
Some UTF-8 filenames can crash smbd and to apply this patch,
  created by MORIYAMA, Masayuki <msyk@mtg.biglobe.ne.jp>, 
  re-found by SATOH Fumiyasu <fumiya@samba.gr.jp>,
  smbd does not crash at the same circumstance.

--- cut here ---
--- smbd/open.c.orig    2003-06-08 02:57:39.000000000 +0900
+++ smbd/open.c 2003-07-03 20:01:37.000000000 +0900
@@ -72,7 +72,7 @@
 static void check_for_pipe(char *fname)
 {
        /* special case of pipe opens */
-       char s[10];
+       pstring s;
        StrnCpy(s,fname,sizeof(s)-1);
        strlower(s);
        if (strstr(s,"pipe/")) {
--- cut here ---


(gdb) show args
Argument list to give program being debugged when it is started is
    "-i -s /usr/local/etc/smb.conf".
(gdb) bt
#0  0x081bc2f4 in strlower_m (s=0xbfbfdfe4 "226ˆä\226", maxlen=6)
    at lib/util_str.c:1456
#1  0x080b75c6 in check_for_pipe (
    fname=0xbfbfe5b0 "tmp/226ˆä\226\207233.mpp") at smbd/open.c:76
#2  0x080b78db in open_file (fsp=0xbfbfe5b0, conn=0x831e800,
    fname=0xbfbfe5b0 "tmp/226ˆä\226\207233.mpp", psbuf=0xbfbfe550,
    flags=0, mode=484, desired_access=131209) at smbd/open.c:179
#3  0x080b9b4c in open_file_shared1 (conn=0x831e800,
    fname=0xbfbfe5b0 "tmp/226ˆä\226\207233.mpp", psbuf=0xbfbfe550,
    desired_access=131209, share_mode=64, ofun=1, new_dos_mode=0,
    oplock_request=3, Access=0xbfbfe13c, paction=0xbfbfe140)
    at smbd/open.c:1250
#4  0x08091f3f in reply_ntcreate_and_X (conn=0x831e800, inbuf=0x82da000 "",
    outbuf=0x82fb000 "", length=116, bufsize=131072) at smbd/nttrans.c:823
#5  0x080ca835 in switch_message (type=162, inbuf=0x82da000 "",
    outbuf=0x82fb000 "", size=116, bufsize=-1077944376) at smbd/process.c:968
#6  0x080ca8fe in construct_reply (inbuf=0x82da000 "", outbuf=0x82fb000 "",
    size=-1077944376, bufsize=-1077944376) at smbd/process.c:998
#7  0x080caccf in process_smb (inbuf=0x82da000 "", outbuf=0x82fb000 "")
    at smbd/process.c:1098
#8  0x080cb953 in smbd_process () at smbd/process.c:1560
#9  0x0822334f in main (argc=-1077944376, argv=0xbfbfec00) at smbd/server.c:910
#10 0x08070652 in _start ()

----- smb.conf
[global]
  dos charset = CP932
  unix charset = UTF-8
  ...
-----
Comment 19 Gerald (Jerry) Carter 2005-02-05 07:57:33 UTC
please retest against 3.0.11 and reopen if necessary.  Also reset 
the version if you reopen the bug report.  Thanks.
Comment 20 Gerald (Jerry) Carter 2005-08-24 10:19:54 UTC
sorry for the same, cleaning up the database to prevent unecessary reopens of bugs.