Fair enough, just wasn't sure what the intended scope was. Thanks for clarifying.
Evan On Mon, Dec 9, 2013 at 8:44 PM, Maynard, Chris <[email protected]> wrote: > I don't think it can be handled in one place (but I could be wrong). A > significant number of dissectors don't use the data parameter at all, so it's > perfectly acceptable for it to be NULL. And while most dissectors that do > expect data either can't deal with a NULL parameter or don't make any effort > to deal with that case, there were some dissectors that did, such as > packet-btl2cap.c (see > http://anonsvn.wireshark.org/viewvc?revision=53784&view=revision). > > I started looking at this to try to avoid more bugs/crashes like > https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=9482. In this case, the > crash was the result of fuzz testing, where the Fibre Channel dissector would > not normally have been called in the first place, so I don't think there's > really anything else that could be done except for adding the extra checks. > > - Chris > > -----Original Message----- > From: [email protected] > [mailto:[email protected]] On Behalf Of Evan Huus > Sent: Monday, December 09, 2013 6:19 PM > To: Wireshark Developer List > Subject: Re: [Wireshark-dev] [Wireshark-commits] rev 53895: /trunk/ > /trunk/asn1/disp/: packet-disp-template.c /trunk/epan/dissectors/: > packet-disp.c packet-dop.c packet-dsp.c packet-hci_usb.c packet-p1.c > packet-pw-atm.c packet-rfid-pn532-hci.c ... > > Should all of these null checks be handled in one place (like > call_dissector_through_handle or something)? > > Are there specific dissectors where it's valid for data to be NULL? > Even if there are, is it simply less work at this point to pass them a > pointer to an empty struct or some such thing? > > Evan > > On Mon, Dec 9, 2013 at 5:23 PM, <[email protected]> wrote: >> http://anonsvn.wireshark.org/viewvc/viewvc.cgi?view=rev&revision=53895 >> >> User: cmaynard >> Date: 2013/12/09 10:23 PM >> >> Log: >> Reject the packet if data is NULL without doing anything else. >> >> Note: We *might* want to do _something_ but that _something_ should be >> well-defined and consistent across all dissectors. Previously, some >> dissectors called proto_tree_add_text() to add some error message text to >> the tree, while others called DISSECTOR_ASSERT(). >> >> Directory: /trunk/asn1/disp/ >> Changes Path Action >> +4 -10 packet-disp-template.c Modified >> >> Directory: /trunk/epan/dissectors/ >> Changes Path Action >> +7 -13 packet-disp.c Modified >> +8 -14 packet-dop.c Modified >> +8 -14 packet-dsp.c Modified >> +6 -3 packet-hci_usb.c Modified >> +8 -14 packet-p1.c Modified >> +12 -4 packet-pw-atm.c Modified >> +6 -3 packet-rfid-pn532-hci.c Modified >> +6 -3 packet-rfid-pn532.c Modified >> +7 -12 packet-ros.c Modified >> +7 -13 packet-rtse.c Modified >> >> >> (6 files not shown) > CONFIDENTIALITY NOTICE: The information contained in this email message is > intended only for use of the intended recipient. If the reader of this > message is not the intended recipient, you are hereby notified that any > dissemination, distribution or copying of this communication is strictly > prohibited. If you have received this communication in error, please > immediately delete it from your system and notify the sender by replying to > this email. Thank you. > ___________________________________________________________________________ > Sent via: Wireshark-dev mailing list <[email protected]> > Archives: http://www.wireshark.org/lists/wireshark-dev > Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev > mailto:[email protected]?subject=unsubscribe ___________________________________________________________________________ Sent via: Wireshark-dev mailing list <[email protected]> Archives: http://www.wireshark.org/lists/wireshark-dev Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev mailto:[email protected]?subject=unsubscribe
