Wireshark-bugs: [Wireshark-bugs] [Bug 8787] 9P dissector - compute fids on first visit

Date: Thu, 01 Aug 2013 05:50:58 +0000

Comment # 20 on bug 8787 from
(In reply to comment #18)
> Yes, tcp_dissect_pdus() is probably what you want to use to handle, well,
> TCP being TCP.

Do you want it in this patch or can it wait for another ticket?

> Another thing which has changed since this started: the ep_ and se_
> allocators have been deprecated.  Ideally those allocations would be
> replaced with wmem (see doc/README.wmem).

wmem looks nice! It doesn't seem such a hard change, but I'd rather have it in
a different patch as this one already is quite big, and I'm more of the
"incremental" type :P (It'd go with the tcp_dissect_pdus one that should be
quite small)
Also, I suppose this means I probably should replace g_malloc with wmem with
the epan scope? does tvb allocated data gets allocated with this as well, and
should use wmem_free instead of g_free? (e.g. tvb_get_string returns a
g_malloc'd string at the moment, I suppose there would be two functions laying
around while the transition is being done?)


> This change shouldn't be applied:
> 
>         struct _9p_hashval *val;
>         val = (struct _9p_hashval *)g_malloc(sizeof(struct _9p_hashval));
> 
> -       val->data = ""
> +       if (len)
> +               val->data = ""
> 
> Since with it val->data could end up uninitialized.  (g_malloc() will return
> NULL if len==0.)

Right.


> I suspect the problem is coming from val->data either being an se_tree or a
> _9p_version and it's getting mixed up.

Indeed, if the data is completely random, a "malformed" conv_set_fid or
conv_set_tag can override the version, and conv_get_fid/conv_get_tag can fetch
it.
I'll add sanity checks to forbid that and do nothing/return not found value if
this happens.


You are receiving this mail because:
  • You are watching all bug changes.