Wireshark-dev: Re: [Wireshark-dev] Replacing wmem_packet_scope() with pinfo->pool?

From: "Dr. Matthias St. Pierre" <Matthias.St.Pierre@xxxxxxxxx>
Date: Mon, 12 Jul 2021 16:41:51 +0000
Sorry Pascal, I overlooked that you already mentioned this difference at the end of your reply:

> I had the same idea in the past, mostly because of subtle bugs where Wireshark was using already freed packet memory because of the
> difference between packet and pool scopes (that is documented in README.wmem but still error prone). ...


> -----Original Message-----
> From: Wireshark-dev <wireshark-dev-bounces@xxxxxxxxxxxxx> On Behalf Of Dr. Matthias St. Pierre
> Sent: Monday, July 12, 2021 6:38 PM
> To: Developer support list for Wireshark <wireshark-dev@xxxxxxxxxxxxx>
> Subject: Re: [Wireshark-dev] Replacing wmem_packet_scope() with pinfo->pool?
> 
> Hi Evan and Pascal,
> 
> > > At a first glance, we already have pinfo->pool which maintains the
> > > lifetime of the packet_info object. As far as I can reason, this is
> > > almost/effectively the same as the existing wmem_packet_scope - it
> > > gets cleaned up later in the dissection flow, but there's still only
> > > ever one which gets reused for each packet.
> >
> > That's also my understanding.
> 
> FWIW: Incidentally, I asked myself the same question only recently, because I noticed that packet-isakmp dissector used
> wmem_packet_scope() in most locations, except for the following two locations, where it allocates memory for the decrypted
> packets:
> 
> https://gitlab.com/wireshark/wireshark/-/blob/86e2fda11e199b8d0e874147e60a1ba1f0ddb803/epan/dissectors/packet-
> isakmp.c#L2355
> https://gitlab.com/wireshark/wireshark/-/blob/86e2fda11e199b8d0e874147e60a1ba1f0ddb803/epan/dissectors/packet-
> isakmp.c#L5960
> 
> Finally, I found the following explanation in README.wmem, which states that the scope of the pinfo pool is slightly larger than the
> packet scope:
> 
> > 2.3 The Pinfo Pool
> 
> > Certain allocations (such as AT_STRINGZ address allocations and anything that
> > might end up being passed to add_new_data_source) need their memory to stick
> > around a little longer than the usual packet scope - basically until the
> > next packet is dissected. This is, in fact, the scope of Wireshark's pinfo
> > structure, so the pinfo struct has a 'pool' member which is a wmem pool scoped
> > to the lifetime of the pinfo struct.
> 
> https://gitlab.com/wireshark/wireshark/-/blob/86e2fda11e199b8d0e874147e60a1ba1f0ddb803/doc/README.wmem#L74-81
> 
> 
> Matthias

Attachment: smime.p7s
Description: S/MIME cryptographic signature