All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bart Westgeest <bart@elbrys.com>
To: linux-bluetooth@vger.kernel.org
Subject: Feedback on potential fix for issue while advertising Feature List
Date: Thu, 08 Nov 2012 10:34:27 -0500	[thread overview]
Message-ID: <509BD103.2040808@elbrys.com> (raw)

Hi folks,

I've tracked down the root cause of an issue, and am looking for 
input/feedback on what the acceptable/preferred fix is.

The code of interest is in sdp_set_supp_feat. The bug is exposed by 
registering a feature list (SDP_ATTR_SUPPORTED_FEATURES_LIST) of a 
length greater than 256. The attribute sequence type in one of the 
resulting attribute sequences is hard-coded to be SDP_SEQ8. When the 
list grows beyond 256 bytes, the size descriptor type is not corrected 
to SDP_SEQ16.

Henceforth, the indicated size of this attribute sequence in an SDP 
response PDU is whatever the lower byte of the feature list size is. 
However, the complete list data is written in the SDP response, hence 
forming a corrupted PDU.

This bug can easily be reproduced by registering a number of Health 
Applications (including a description string) such that the size of the 
advertised feature list attribute is greater than 256 bytes. 
Subsequently querying for this SDP record with sdptool, will result in 
indicated errors about unknown data due to parsing errors.

The quick and dirty fix is to change the return statement in 
sdp_seq_alloc (lib/sdp.c:563) from:
	return sdp_data_alloc(SDP_SEQ8, seq);
to
	return sdp_data_alloc(SDP_SEQ16, seq);

However, it really looks like the data type should be determined 
dynamically, or perhaps (a more involved fix,) instead of using nested 
data sequences for the outer data type, perhaps it should be changed to 
a list i.e. to use sdp_list_t instead sdp_data_t for the feature list.

In addition, this problem (hard coded sequence length for a variable 
length list) seems be repeated at various places throughout the code, so 
this problem does not appear to be isolated to sdp_set_supp_feat/HDP. 
For example sdp_set_lang_attr, sdp_set_uuidseq_attr, and 
sdp_set_profile_descs all appear to form corrupted PDU if their 
specified list grows beyond 256.

I appreciate any input from anybody more familiar with the SDP code on 
what the accepted/preferred approach is to get this issue fixed.

Thanks,
Bart

             reply	other threads:[~2012-11-08 15:34 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-11-08 15:34 Bart Westgeest [this message]
2012-11-09 22:45 ` Feedback on potential fix for issue while advertising Feature List Bart Westgeest
2012-11-10 12:25   ` Johan Hedberg
2012-11-12 19:36     ` Bart Westgeest

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=509BD103.2040808@elbrys.com \
    --to=bart@elbrys.com \
    --cc=linux-bluetooth@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.