Actions
Bug #5111
closedsmb_common_rename uses uninitialized variable
Status:
Closed
Priority:
Normal
Assignee:
-
Category:
cifs - CIFS server and client
Start date:
2014-08-21
Due date:
% Done:
100%
Estimated time:
Difficulty:
Medium
Tags:
needs-triage
Gerrit CR:
External Bug:
Description
An OmniOS customer reported some SMB coredumps. The first of which was this:
debugging crash dump vmcore.0 (64-bit) from Olympus operating system: 5.11 omnios-8c08411 (i86pc) image uuid: 331b57b0-7aa4-e892-b710-ae1c59c6fe6a panic message: BAD TRAP: type=e (#pf Page fault) rp=ffffff0011a03450 addr=ffffff03197b10c8 dump content: kernel pages only > $c smb_fsop_lookup+0x118(ffffff03197b0990, ffffff02dbfca018, 0, ffffff03197b0db8, ffffff02da252970, ffffff03197b0ea8) smb_common_rename+0xd9(ffffff03197b0990, ffffff03197b0bc8, ffffff03197b0db8) smb_trans2_rename+0x136(ffffff03197b0990, ffffff02e60c9ca0, ffffff02dc90b628, 1 ) smb_set_rename_info+0xb8(ffffff03197b0990, ffffff0011a03840) smb_set_fileinfo+0xed(ffffff03197b0990, ffffff0011a03840) smb_set_by_fid+0xb0(ffffff03197b0990, ffffff030f2ff400, 3f2) smb_com_trans2_set_file_information+0x58(ffffff03197b0990, ffffff030f2ff400) smb_trans2_dispatch+0x313(ffffff03197b0990, ffffff030f2ff400) smb_com_transaction2+0x1a7(ffffff03197b0990) smb_dispatch_request+0x662(ffffff03197b0990) smb_session_worker+0xa0(ffffff03197b0990) taskq_d_thread+0xb7(ffffff0331430760) thread_start+8()
The faulty addr looks like arg3 (4th one) to smb_fsop_lookup(). The calling function calls smb_fsop_lookup in two places. Both use local variable "tnode" for it.
Only one of them, however, has tnode initialized. Here's a subset of the code.
smb_node_t *tnode; /* XXX KEBE SAYS UNINITIALIZED */ . . . /* Find destination dnode and last_comp */ 352 if (dst_fqi->fq_dnode) { /* XXX WHAT IF I TAKE THIS PATH... */ 353 smb_node_ref(dst_fqi->fq_dnode); 354 } else { 355 tnode = sr->tid_tree->t_snode; 356 rc = smb_pathname_reduce(sr, sr->user_cr, path, tnode, tnode, 357 &dst_fqi->fq_dnode, dst_fqi->fq_last_comp); 358 if (rc != 0) { 359 smb_rename_release_src(sr); 360 return (rc); 361 } 362 } . . . /* AND HIT HERE?!?!? */ 375 /* Lookup destination node */ 376 rc = smb_fsop_lookup(sr, sr->user_cr, 0, tnode, 377 dst_dnode, new_name, &dst_fqi->fq_fnode);
Consultation with CIFS expert Gordon Ross suggests we move the initialization of "tnode" out of the else-clause, and have it always initialize.
Updated by Electric Monk almost 9 years ago
- Status changed from New to Closed
- % Done changed from 80 to 100
git commit 896cab575ec8a9c845350d470dabb5a0997efebf
commit 896cab575ec8a9c845350d470dabb5a0997efebf Author: Dan McDonald <danmcd@omniti.com> Date: 2014-08-21T18:28:27.000Z 5111 smb_common_rename uses uninitialized variable Reviewed by: Richard Lowe <richlowe@richlowe.net> Reviewed by: Thomas Keiser <tkeiser@gmail.com> Reviewed by: Andy Stormont <AStormont@racktopsystems.com> Approved by: Gordon Ross <gwr@nexenta.com>
Actions