Re: LUA chained dissector drops data parameter

[Michael Mann via Wireshark-dev <wireshark-dev () wireshark org>]

Using proto data "works" and I believe is used in a few places, but is not a preferred solution because it makes 
"dissector flow" much harder to follow and maintenance more difficult.  I have spend some time converting dissectors to 
pass dissector data directly instead of using proto data. I'm not familiar with the Bluetooth code, but it seems 
expansive enough that using proto data could more easily need to dissection bugs.



-----Original Message-----
From: Kanstrup, Mikael <Mikael.Kanstrup () sony com>
To: Peter Wu <peter () lekensteyn nl>; Developer support list for Wireshark <wireshark-dev () wireshark org>
Sent: Fri, Nov 29, 2019 10:26 am
Subject: Re: [Wireshark-dev] LUA chained dissector drops data parameter

#yiv6376871419 P {margin-top:0;margin-bottom:0;}Hi Peter,
Thanks a lot for an extensive answer with links to related discussions! 
Have proto data use been considered for passing data around instead? I suppose there's a performance penalty to using 
that over directly passing the pointer. For my scenario the data parameter can quite easily be replaced with 
p_get_proto_data / p_add_proto_data. This works for cases where the LUA script itself does not need access the data 
passed around.
I uploaded a change for this discussion here. I have not done any extensive testing but it shows the 
idea:https://code.wireshark.org/review/35260

Performance wise using callgrind with tshark -V on my packet capture file with 16000 packets I get around 0.5% higher 
numbers with proto data compared to directly passing the pointer.
/Mikael
Från: Wireshark-dev <wireshark-dev-bounces () wireshark org> för Peter Wu <peter () lekensteyn nl>
Skickat: den 26 november 2019 01:40
Till: Developer support list for Wireshark <wireshark-dev () wireshark org>
Ämne: Re: [Wireshark-dev] LUA chained dissector drops data parameter Hi Mikael,

On Mon, Nov 18, 2019 at 05:20:54PM +0000, Kanstrup, Mikael wrote:
> Hi,
>
> I'm working on dissecting a proprietary protocol that extends Bluetooth HCI_ACL with a LUA dissector. As there's no 
> heuristics dissector list registered for this particular protocol I thought something similar could be achieved with 
> a chained dissector. I retrieve the original HCI_ACL dissector handle and replace it with my own LUA dissector. In 
> LUA dissector apply some heuristics and if it's not my own protocol then call the original HCI_ACL dissector via the 
> handle.
>
> Code looks like this:
>
> local proto_test = Proto("test", "Use chaining as heuristic dissector")
> local proto_default_acl
>
> function is_test_proto(tvb, pinfo)
>      -- Apply heuristics to determine if own protocol
>      return false
> end
>
> function proto_test.dissector(tvb, pinfo, tree)
>      if not is_test_proto(tvb, pinfo) then
>          return proto_default_acl:call(tvb, pinfo, tree)
>      end
>
>      pinfo.cols.protocol = "test"
>      tree = tree:add(proto_test, tvb)
>      return tvb:len()
> end
>
> function proto_test.init()
>      local hci_type = DissectorTable.get("hci_h4.type")
>      local pattern = 0x02 -- ACL
>      proto_default_acl = hci_type:get_dissector(pattern)
>      hci_type:add(pattern, proto_test)
> end
>
> This unfortunately did not work and I was not able to find out why until I started looking at the HCI_ACL dissector 
> code itself.
>
> static gint
> dissect_bthci_acl(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void *data)
> {
> <...>
>      /* Reject the packet if data is NULL */
>      if (data == NULL)
>          return 0;
>
> The above NULL check is hit for all calls coming from the LUA dissector. The LUA dissector function prototype does 
> not have the data parameter and it appears it's simply lost when chaining calls through LUA.

Correct, Lua always passes a NULL parameter. Due to type-safety,
Lua dissectors cannot pass arbitrary data.

> Any suggestions on how to approach this? Would it be possible to
> extend the LUA dissector interface with another function prototype
> that supports the data parameter? Just support relaying the parameter
> in chained dissectors, not modifying or doing any fancy stuff with it.

There is unfortunately no runtime registration of types for the 'data'
parameter. If you can somehow keep track that a C dissector was from a
particular dissector table, and then recognize that the Lua dissector
was registered with the same table, then hopefully it should be possible
to propagate the data parameter as special case.

Long-term, we might have to revisit how to pass data between (multiple
layers of) dissectors. Not just Lua, but also C. For Lua, see also:

Bug 15931 - Add Lua support for arbitrary data parameter in dissector calls.
https://urldefense.proofpoint.com/v2/url?u=https-3A__bugs.wireshark.org_bugzilla_show-5Fbug.cgi-3Fid-3D15931&d=DwIGaQ&c=fP4tf--1dS0biCFlB0saz0I0kjO5v7-GLPtvShAo4cc&r=W4de0lx_38xjApzeLJAbvbv2_hc2Iu3XIsdzy2qOqXI&m=H9rWMwiJyfh8eIuBmZCEEPnMb_oP3346TmGdwbolF00&s=eVevrJK65E3vVrpXCTbmszr11My9pHoJCnjOqUBRqmE&e=

(+cc Huang)
Discussion about another mechanism to pass data between dissectors:
https://urldefense.proofpoint.com/v2/url?u=https-3A__code.wireshark.org_review_35159&d=DwIGaQ&c=fP4tf--1dS0biCFlB0saz0I0kjO5v7-GLPtvShAo4cc&r=W4de0lx_38xjApzeLJAbvbv2_hc2Iu3XIsdzy2qOqXI&m=H9rWMwiJyfh8eIuBmZCEEPnMb_oP3346TmGdwbolF00&s=I1OYFpp6Q0WPJl8x_KX5XQEAg5PrDYT7y78XNVzkr6A&e=

Discussion about another (abandoned) new mechanism to pass data between
dissectors:
https://urldefense.proofpoint.com/v2/url?u=https-3A__code.wireshark.org_review_34049&d=DwIGaQ&c=fP4tf--1dS0biCFlB0saz0I0kjO5v7-GLPtvShAo4cc&r=W4de0lx_38xjApzeLJAbvbv2_hc2Iu3XIsdzy2qOqXI&m=H9rWMwiJyfh8eIuBmZCEEPnMb_oP3346TmGdwbolF00&s=ALrCftdEain-8AfSa62fClqocWJBCrn4f0Jp5TcgeVA&e=
-- 
Kind regards,
Peter Wu
https://urldefense.proofpoint.com/v2/url?u=https-3A__lekensteyn.nl&d=DwIGaQ&c=fP4tf--1dS0biCFlB0saz0I0kjO5v7-GLPtvShAo4cc&r=W4de0lx_38xjApzeLJAbvbv2_hc2Iu3XIsdzy2qOqXI&m=H9rWMwiJyfh8eIuBmZCEEPnMb_oP3346TmGdwbolF00&s=z9X2kmqTKS7mT6tjqQRWOLSAd_UxvGL34mLbje-dMRo&e=
___________________________________________________________________________
Sent via:    Wireshark-dev mailing list <wireshark-dev () wireshark org>
Archives:    
https://urldefense.proofpoint.com/v2/url?u=https-3A__www.wireshark.org_lists_wireshark-2Ddev&d=DwIGaQ&c=fP4tf--1dS0biCFlB0saz0I0kjO5v7-GLPtvShAo4cc&r=W4de0lx_38xjApzeLJAbvbv2_hc2Iu3XIsdzy2qOqXI&m=H9rWMwiJyfh8eIuBmZCEEPnMb_oP3346TmGdwbolF00&s=OC3ix68XPKvNP4K1ir8q2NZpMSVRi7D8_b27PTlhIFI&e=
Unsubscribe: 
https://urldefense.proofpoint.com/v2/url?u=https-3A__www.wireshark.org_mailman_options_wireshark-2Ddev&d=DwIGaQ&c=fP4tf--1dS0biCFlB0saz0I0kjO5v7-GLPtvShAo4cc&r=W4de0lx_38xjApzeLJAbvbv2_hc2Iu3XIsdzy2qOqXI&m=H9rWMwiJyfh8eIuBmZCEEPnMb_oP3346TmGdwbolF00&s=mzAKn9vjZnnXtcIl7ni_9yEy177WxYVXnEAZXXAzlZ0&e=
             mailto:wireshark-dev-request () wireshark 
org?subject=unsubscribe___________________________________________________________________________
Sent via:    Wireshark-dev mailing list <wireshark-dev () wireshark org>
Archives:    https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
            mailto:wireshark-dev-request () wireshark org?subject=unsubscribe

___________________________________________________________________________
Sent via:    Wireshark-dev mailing list <wireshark-dev () wireshark org>
Archives:    https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
             mailto:wireshark-dev-request () wireshark org?subject=unsubscribe