[Guy Harris <guy@xxxxxxxxxx>]

On Wed, Feb 12, 2003 at 11:46:37PM +0100, Jochen Friedrich wrote:
> this patch updates the SNA dissector code:

Checked in, with one change:

> Index: packet-llc.c
> ===================================================================
> RCS file: /cvsroot/ethereal/packet-llc.c,v
> retrieving revision 1.106
> diff -u -r1.106 packet-llc.c
> --- packet-llc.c	25 Jan 2003 00:06:12 -0000	1.106
> +++ packet-llc.c	12 Feb 2003 22:15:34 -0000
> @@ -65,6 +65,7 @@
>  
>  static dissector_table_t subdissector_table;
>  static dissector_table_t cisco_subdissector_table;
> +static dissector_table_t xid_subdissector_table;
>  
>  static dissector_handle_t bpdu_handle;
>  static dissector_handle_t eth_handle;
> @@ -322,16 +323,26 @@
>  			);
>  		}
>  
> -		next_tvb = tvb_new_subset(tvb, llc_header_len, -1, -1);
> -		if (XDLC_IS_INFORMATION(control)) {
> -			/* non-SNAP */
> -			/* do lookup with the subdissector table */
> -			if (!dissector_try_port(subdissector_table, dsap,
> -			    next_tvb, pinfo, tree)) {
> -				call_dissector(data_handle,next_tvb, pinfo, tree);
> +		if (tvb_offset_exists(tvb, llc_header_len + 1)) {
> +			next_tvb = tvb_new_subset(tvb, llc_header_len, -1, -1);
> +			if (XDLC_IS_INFORMATION(control)) {
> +				/* non-SNAP */
> +				/* do lookup with the subdissector table */
> +				if (!dissector_try_port(subdissector_table,
> +				    dsap, next_tvb, pinfo, tree)) {
> +					call_dissector(data_handle,next_tvb,
> +					    pinfo, tree);
> +				}
> +			} else if ((control & 0xef) == (XDLC_XID|XDLC_U)) {
> +				if (!dissector_try_port(xid_subdissector_table,
> +				    dsap, next_tvb, pinfo, tree)) {
> +					call_dissector(data_handle,next_tvb,
> +					    pinfo, tree);
> +				}
> +			} else {
> +				call_dissector(data_handle,next_tvb, pinfo,
> +				    tree);
>  			}
> -		} else {
> -			call_dissector(data_handle,next_tvb, pinfo, tree);
>  		}
>  	}
>  }

I checked for "tvb_remaining_length(tvb, llc_header_len)" instead -
checking "tvb_offset_exists(tvb, llc_header_len + 1)" requires that
there be at least *two* bytes starting at "llc_header_len" (the one at
"llc_header_len" and the one at "llc_header_len + 1"), but one
should be sufficient.

The same should perhaps be done in the SNA dissector:

> +	while (tvb_offset_exists(tvb, offset+1)) {
> +		len = tvb_get_guint8(tvb, offset+0);
> +		if (len) {
> +			dissect_control(tvb_new_subset(tvb, offset, len, -1),
> +			    tree, 1, LT);
> +			pad = (len+3) & 0xfffc;
> +			if (pad > len)
> +				proto_tree_add_text(tree, tvb, offset+len,
> +				    pad-len, "Padding");
> +			offset += pad;
> +		} else {
> +			return;
> +		}
> +	}
>  }

> +		if (tvb_offset_exists(tvb, offset+1))
> +			call_dissector(data_handle,
> +			    tvb_new_subset(tvb, 4, -1, -1), pinfo, tree);

> +	proto_tree_add_item(tree, hf_sna_nlp_opti_0f_bits, tvb, 2, 2, FALSE);
> +	if (tvb_offset_exists(tvb, 5))
> +		call_dissector(data_handle,
> +		    tvb_new_subset(tvb, 4, -1, -1), pinfo, tree);

> +	if (tvb_offset_exists(tvb, 13))
> +		call_dissector(data_handle,
> +		    tvb_new_subset(tvb, 12, -1, -1), pinfo, tree);

> +		if (tvb_offset_exists(tvb, 21))
> +			call_dissector(data_handle,
> +			    tvb_new_subset(tvb, 20, -1, -1), pinfo, tree);
> +	} else {
> +		if (tvb_offset_exists(tvb, 13))
> +			call_dissector(data_handle,
> +			    tvb_new_subset(tvb, 12, -1, -1), pinfo, tree);

> +	while (tvb_offset_exists(tvb, offset+1)) {
> +		len = tvb_get_guint8(tvb, offset);
> +		type = tvb_get_guint8(tvb, offset+1);

> +
> +			if (tvb_offset_exists(tvb, index+1))
> +				call_dissector(data_handle,
> +					tvb_new_subset(tvb, index, -1, -1),
> +					pinfo, parent_tree);

> +		if (tvb_offset_exists(tvb, index+1)) {
> +			call_dissector(data_handle,
> +			    tvb_new_subset(tvb, index, -1, -1), pinfo,
> +			    parent_tree);
> +		}

and so on.