Samba working again

Submitted by Robert Szeleney on Wed, 2006-06-28 15:05.

Finally the samba bug has been fixed. Following mail to the samba mailing list shows what was wrong:

-----------------

Hi!

I think there is a little bug in the current samba release (3.0.22).

Take a look at cp850.c at the last line and you will see following
definition:

SMB_GENERATE_CHARSET_MODULE_8_BIT_GAP(CP850)

Using the macros from charset.c, the preprocessor expands this to:

NTSTATUS charset_CP850_init(void) \
{ \
return smb_register_charset(&CP850_functions); \
} \

NTSTATUS is defined in nt_status.h as follows:

typedef struct {uint32 v;} NTSTATUS;
#define NT_STATUS(x) ((NTSTATUS) { x })
#define NT_STATUS_V(x) ((x).v)

Ok, when cp850/cp437 is compiled as static, config.h has following macro:
#define static_init_charset { charset_CP850_init();
charset_CP437_init();}

This static_init_charset macro gets called in iconv.c. But at this point
in the file there is no function prototype for charset_CP850_init and
charset_CP437_init. Thus gcc, doesn't know that it has to reserve space on
the stack for this 4 byte return value which actually gets returned AND
pushed to the stack by charset_CP850_init.

After adding following prototype to config.h, everything works as
expected:
NT_STATUS charset_CP850_init(void);

You may look at this disassembly which shows whats going wrong exactely:

iconv.c: smb_iconv_open(): (With the NT_STATUS charset_CP850_init(void);
prototype:)

807c173: 85 c0 test %eax,%eax
807c175: 75 e9 jne 807c160

807c177: 8d 45 ec lea 0xffffffec(%ebp),%eax
807c17a: 83 ec 0c sub $0xc,%esp
807c17d: 50 push %eax // here gcc reserves
space on the stack
807c17e: e8 b9 96 00 00 call 808583c

807c183: 83 c4 0c add $0xc,%esp
807c186: e8 7d 98 00 00 call 8085a08

iconv.c: smb_iconv_open(): (Without the NT_STATUS
charset_CP850_init(void); prototype:)

807c173: 85 c0 test %eax,%eax
807c175: 75 e9 jne 807c160

807c177: e8 b8 96 00 00 call 8085834
// no space reserved
807c17c: e8 7f 98 00 00 call 8085a00

cp850.c : charset_CP850_init
808583c: 55 push %ebp
808583d: 89 e5 mov %esp,%ebp
808583f: 56 push %esi
8085840: 53 push %ebx
8085841: 83 ec 18 sub $0x18,%esp
8085844: e8 00 00 00 00 call 8085849

8085849: 5b pop %ebx
808584a: 81 c3 8f 1a 03 00 add $0x31a8f,%ebx
8085850: 8b 75 08 mov 0x8(%ebp),%esi // read
NT_STATUS
8085853: 8d 55 f4 lea 0xfffffff4(%ebp),%edx
8085856: 8d 83 ac 60 00 00 lea 0x60ac(%ebx),%eax
808585c: 50 push %eax
808585d: 52 push %edx
808585e: e8 73 67 ff ff call 807bfd6

8085863: 8b 45 f4 mov 0xfffffff4(%ebp),%eax
8085866: 89 06 mov %eax,(%esi) // write
NT_STATUS
8085868: 89 f0 mov %esi,%eax
808586a: 8d 65 f8 lea 0xfffffff8(%ebp),%esp
808586d: 5b pop %ebx
808586e: 5e pop %esi
808586f: 5d pop %ebp
8085870: c2 04 00 ret $0x4
8085873: 90 nop

As you can see,
8085866: 89 06 mov %eax,(%esi) // write
NT_STATUS
tries to write the error code to the address pushed on the stack before,
which actually never happened when the prototype is missing.

Btw, this is compiled with GCC 4.1.1 / ELF / PIC

May I be wrong here? Do you need any additional information?

Thanks,
Robert!



MUmjVR

dEoCVQ MUmjVR

WcCHKkU

vNhEtJvO WcCHKkU