Bug 9513 - SEGV when using vfs_full_audit and accessing msdfs link
SEGV when using vfs_full_audit and accessing msdfs link
Status: ASSIGNED
Product: Samba 3.6
Classification: Unclassified
Component: VFS Modules
3.6.10
All All
: P5 major
: ---
Assigned To: Jeremy Allison
Samba QA Contact
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-12-19 11:09 UTC by Tsukasa HAMANO
Modified: 2013-02-07 08:40 UTC (History)
3 users (show)

See Also:


Attachments
patch for v3-6-test (1.37 KB, patch)
2012-12-19 11:12 UTC, Tsukasa HAMANO
no flags Details
Different patch for 3.6.x. (2.09 KB, patch)
2012-12-20 01:07 UTC, Jeremy Allison
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Tsukasa HAMANO 2012-12-19 11:09:42 UTC
Reproduce conditions are:
- vfs objects = full_audit
- access to msdfs link

The reason of this issue is from NULL reference at smb_full_audit_connect() when accessing to msdfs link.

in vfs_full_audit.c:audit_prefix():
result = talloc_sub_advanced(ctx,
                lp_servicename(SNUM(conn)),
                conn->session_info->unix_name, // <- conn->session_info is NULL

How about using current_user.conn->session_info in msdfs case

  BACKTRACE: 30 stack frames:
   #0 /usr/local/samba-3.6/sbin/smbd(log_stack_trace+0x2b) [0xb71266ce]
   #1 /usr/local/samba-3.6/sbin/smbd(smb_panic+0x7f) [0xb712653d]
   #2 /usr/local/samba-3.6/sbin/smbd(+0x4beef8) [0xb7113ef8]
   #3 /usr/local/samba-3.6/sbin/smbd(+0x4bef09) [0xb7113f09]
   #4 [0xb6c37400]
   #5 /usr/local/samba-3.6/lib/vfs/full_audit.so(+0x3dcf) [0xb6627dcf]
   #6 /usr/local/samba-3.6/lib/vfs/full_audit.so(+0x412c) [0xb662812c]
   #7 /usr/local/samba-3.6/sbin/smbd(smb_vfs_call_connect+0x3a) [0xb6d9444f]
   #8 /usr/local/samba-3.6/sbin/smbd(create_conn_struct+0x458) [0xb6dbab3d]
   #9 /usr/local/samba-3.6/sbin/smbd(get_referred_path+0x52d) [0xb6dbc4bb]
   #10 /usr/local/samba-3.6/sbin/smbd(setup_dfs_referral+0x18e) [0xb6dbd263]
   #11 /usr/local/samba-3.6/sbin/smbd(+0x123339) [0xb6d78339]
   #12 /usr/local/samba-3.6/sbin/smbd(+0x123dc5) [0xb6d78dc5]
   #13 /usr/local/samba-3.6/sbin/smbd(reply_trans2+0x8c1) [0xb6d7976a]
   #14 /usr/local/samba-3.6/sbin/smbd(+0x153776) [0xb6da8776]
   #15 /usr/local/samba-3.6/sbin/smbd(+0x153901) [0xb6da8901]
   #16 /usr/local/samba-3.6/sbin/smbd(+0x153c35) [0xb6da8c35]
   #17 /usr/local/samba-3.6/sbin/smbd(+0x154f32) [0xb6da9f32]
   #18 /usr/local/samba-3.6/sbin/smbd(+0x154fa8) [0xb6da9fa8]
   #19 /usr/local/samba-3.6/sbin/smbd(run_events_poll+0x62d) [0xb713a096]
   #20 /usr/local/samba-3.6/sbin/smbd(+0x152cc3) [0xb6da7cc3]
   #21 /usr/local/samba-3.6/sbin/smbd(smbd_process+0xc70) [0xb6dac6d8]
   #22 /usr/local/samba-3.6/sbin/smbd(+0x8ca88e) [0xb751f88e]
   #23 /usr/local/samba-3.6/sbin/smbd(run_events_poll+0x62d) [0xb713a096]
   #24 /usr/local/samba-3.6/sbin/smbd(+0x4e5326) [0xb713a326]
   #25 /usr/local/samba-3.6/sbin/smbd(_tevent_loop_once+0xdd) [0xb713b298]
   #26 /usr/local/samba-3.6/sbin/smbd(+0x8cb400) [0xb7520400]
   #27 /usr/local/samba-3.6/sbin/smbd(main+0x10d7) [0xb752154f]
   #28 /lib/i686/cmov/libc.so.6(__libc_start_main+0xe6) [0xb6a06ca6]
   #29 /usr/local/samba-3.6/sbin/smbd(+0xb3771) [0xb6d08771]
Comment 1 Tsukasa HAMANO 2012-12-19 11:12:52 UTC
Created attachment 8359 [details]
patch for v3-6-test

patch for v3-6-test
Comment 2 Jeremy Allison 2012-12-20 00:03:05 UTC
I think a better way to fix this is to ensure that conn->session_info cannot be NULL..

Jeremy.
Comment 3 Jeremy Allison 2012-12-20 01:07:50 UTC
Created attachment 8360 [details]
Different patch for 3.6.x.

Can you try this patch instead ? I think it should fix all areas where we're assuming conn->session_info != NULL

Jeremy.
Comment 4 Tsukasa HAMANO 2012-12-20 03:32:32 UTC
(In reply to comment #2)
> I think a better way to fix this is to ensure that conn->session_info cannot be
> NULL..
> 

Actually, I think to same way too.
I'll try fix another way.

Thank you.
Comment 5 Tsukasa HAMANO 2012-12-21 03:15:51 UTC
I've just confirmed your patch, there were no problems also in other vfs module.
Thank you.
Comment 6 Tsukasa HAMANO 2012-12-21 05:30:28 UTC
Hi, your patch looks like there are no problem with DFS connect case,
but I found dubious in other case such as that called from _srvsvc_NetGetFileSecurity()

if (session_info != NULL) {
    vfs_user = conn->session_info->unix_name;// <- May be conn->session_info is NULL
    conn->session_info = copy_serverinfo(conn, session_info);

We need to fix follows:
if (session_info != NULL) {
    conn->session_info = copy_serverinfo(conn, session_info);
    vfs_user = conn->session_info->unix_name;

Thank you.
Comment 7 Tsukasa HAMANO 2012-12-21 05:43:27 UTC
(In reply to comment #6)
> Hi, your patch looks like there are no problem with DFS connect case,
> but I found dubious in other case such as that called from
> _srvsvc_NetGetFileSecurity()
> 
> if (session_info != NULL) {
>     vfs_user = conn->session_info->unix_name;// <- May be conn->session_info is
> NULL
>     conn->session_info = copy_serverinfo(conn, session_info);
> 
> We need to fix follows:
> if (session_info != NULL) {
>     conn->session_info = copy_serverinfo(conn, session_info);
>     vfs_user = conn->session_info->unix_name;
> 
> Thank you.

Or, actually you want to write follows:

if (session_info != NULL) {
    vfs_user = session_info->unix_name;
    conn->session_info = copy_serverinfo(conn, session_info);