Generic code to walk through the fields in a radiotap header, accounting
for nasties like extended "field present" bitfields and alignment rules
Try #2
- added sanity check to extended present bitmap u32 walking code - disallow
possibility to walk past the end of the radiotap header length
Try #1
- Based on Johannes Berg's comments, broke out the radiotap parsing into
its own file as part of cfg80211
- From same comments, added mask constant for b31 of arg presence bitfield
to ieee80211_radiotap.h
- Fixed subtle but nasty bug with alignment: args in the radiotap area are
aligned *relative to the start of the header* now. The header is not
guaranteed to align to anything (it is randomly in an skb data area).
Signed-off-by: Andy Green <[email protected]>
===================================================
---
include/net/cfg80211.h | 47 +++++++++
net/wireless/Makefile | 2
net/wireless/radiotap.c | 236 ++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 284 insertions(+), 1 deletion(-)
Index: wireless-dev/include/net/cfg80211.h
===================================================================
--- wireless-dev.orig/include/net/cfg80211.h
+++ wireless-dev/include/net/cfg80211.h
@@ -74,6 +74,53 @@ struct key_params {
u32 cipher;
};
+
+/* Radiotap header iteration
+ * implemented in net/wireless/radiotap.c
+ *
+ * call __ieee80211_radiotap_iterator_init() to init a semi-opaque iterator
+ * struct ieee80211_radiotap_iterator (no need to init the struct beforehand)
+ * then loop calling __ieee80211_radiotap_iterator_next()... it returns -1
+ * if there are no more args in the header, or the next argument type index
+ * that is present. The iterator's this_arg member points to the start of the
+ * argument associated with the current argument index that is present,
+ * which can be found in the iterator's this_arg_index member. This arg
+ * index corresponds to the IEEE80211_RADIOTAP_... defines.
+ */
+/**
+ * struct ieee80211_radiotap_iterator - tracks walk thru present radiotap args
+ * @rtheader: pointer to the radiotap header we are walking through
+ * @max_length: length of radiotap header in cpu byte ordering
+ * @this_arg_index: IEEE80211_RADIOTAP_... index of current arg
+ * @this_arg: pointer to current radiotap arg
+ * @arg_index: internal next argument index
+ * @arg: internal next argument pointer
+ * @next_bitmap: internal pointer to next present u32
+ * @bitmap_shifter: internal shifter for curr u32 bitmap, b0 set == arg present
+ */
+
+struct ieee80211_radiotap_iterator {
+ struct ieee80211_radiotap_header *rtheader;
+ int max_length;
+ int this_arg_index;
+ u8 * this_arg;
+
+ int arg_index;
+ u8 * arg;
+ __le32 *next_bitmap;
+ u32 bitmap_shifter;
+};
+
+extern int ieee80211_radiotap_iterator_init(
+ struct ieee80211_radiotap_iterator * iterator,
+ struct ieee80211_radiotap_header * radiotap_header,
+ int max_length
+);
+
+extern int ieee80211_radiotap_iterator_next(
+ struct ieee80211_radiotap_iterator * iterator);
+
+
/* from net/wireless.h */
struct wiphy;
Index: wireless-dev/net/wireless/Makefile
===================================================================
--- wireless-dev.orig/net/wireless/Makefile
+++ wireless-dev/net/wireless/Makefile
@@ -1,5 +1,5 @@
obj-$(CONFIG_WIRELESS_EXT) += wext.o
obj-$(CONFIG_CFG80211) += cfg80211.o
-cfg80211-y += core.o sysfs.o
+cfg80211-y += core.o sysfs.o radiotap.o
cfg80211-$(CONFIG_NL80211) += nl80211.o
Index: wireless-dev/net/wireless/radiotap.c
===================================================================
--- /dev/null
+++ wireless-dev/net/wireless/radiotap.c
@@ -0,0 +1,236 @@
+/*
+ * Radiotap parser
+ *
+ * Copyright 2007 Andy Green <[email protected]>
+ */
+
+#include <linux/if.h>
+#include <linux/err.h>
+#include <linux/mutex.h>
+#include <linux/device.h>
+#include <net/genetlink.h>
+#include <net/cfg80211.h>
+#include <net/wireless.h>
+#include "nl80211.h"
+#include "core.h"
+#include <net/ieee80211_radiotap.h>
+
+
+/**
+ * ieee80211_radiotap_iterator_init - radiotap parser iterator initialization
+ * @iterator: radiotap_iterator to initialize
+ * @radiotap_header: radiotap header to parse
+ * @max_length: total length we can parse into (eg, whole packet length)
+ *
+ * Returns: 0 on success or negative if sanity check fails
+ *
+ * This function initializes an opaque iterator struct which can then
+ * be passed to ieee80211_radiotap_iterator_next() to visit every radiotap
+ * argument which is present in the header. It knows about extended
+ * present headers and handles them.
+ */
+
+int ieee80211_radiotap_iterator_init(
+ struct ieee80211_radiotap_iterator * iterator,
+ struct ieee80211_radiotap_header * radiotap_header,
+ int max_length)
+{
+ /* Linux only supports version 0 radiotap format */
+
+ if (radiotap_header->it_version)
+ return -EINVAL;
+
+ /* sanity check for allowed length and radiotap length field */
+
+ if (max_length < (le16_to_cpu(radiotap_header->it_len)))
+ return -EINVAL;
+
+ iterator->rtheader = radiotap_header;
+ iterator->max_length = le16_to_cpu(radiotap_header->it_len);
+ iterator->arg_index = 0;
+ iterator->bitmap_shifter = le32_to_cpu(radiotap_header->it_present);
+ iterator->arg = ((u8 *)radiotap_header) +
+ sizeof(struct ieee80211_radiotap_header);
+ iterator->this_arg = 0;
+
+ /* find payload start allowing for extended bitmap(s) */
+
+ if (unlikely(iterator->bitmap_shifter &
+ IEEE80211_RADIOTAP_PRESENT_EXTEND_MASK)) {
+ while (le32_to_cpu(*((u32 *)iterator->arg)) &
+ IEEE80211_RADIOTAP_PRESENT_EXTEND_MASK) {
+ iterator->arg += sizeof(u32);
+
+ /*
+ * check for insanity where the present bitmaps
+ * keep claiming to extend up to or even beyond the
+ * stated radiotap header length
+ */
+
+ if ((((int)iterator->arg) - ((int)iterator->rtheader)) >
+ iterator->max_length)
+ return -EINVAL;
+ }
+
+ iterator->arg += sizeof(u32);
+
+ /*
+ * no need to check again for blowing past stated radiotap
+ * header length, becuase ieee80211_radiotap_iterator_next
+ * checks it before it is dereferenced
+ */
+ }
+
+ /* we are all initialized happily */
+
+ return 0;
+}
+
+EXPORT_SYMBOL(ieee80211_radiotap_iterator_init);
+
+
+/**
+ * ieee80211_radiotap_iterator_next - return next radiotap parser iterator arg
+ * @iterator: radiotap_iterator to move to next arg (if any)
+ *
+ * Returns: next present arg index on success or negative if no more or error
+ *
+ * This function returns the next radiotap arg index (IEEE80211_RADIOTAP_*)
+ * and sets iterator->this_arg to point to the payload for the arg. It takes
+ * care of alignment handling and extended present fields. interator->this_arg
+ * can be changed by the caller. The args pointed to are in little-endian
+ * format.
+ */
+
+int ieee80211_radiotap_iterator_next(
+ struct ieee80211_radiotap_iterator * iterator)
+{
+
+ /*
+ * small length lookup table for all radiotap types we heard of
+ * starting from b0 in the bitmap, so we can walk the payload
+ * area of the radiotap header
+ *
+ * There is a requirement to pad args, so that args
+ * of a given length must begin at a boundary of that length
+ * -- but note that compound args are allowed (eg, 2 x u16
+ * for IEEE80211_RADIOTAP_CHANNEL) so total arg length is not
+ * a reliable indicator of alignment requirement.
+ *
+ * upper nybble: content alignment for arg
+ * lower nybble: content length for arg
+ */
+
+ static const u8 rt_sizes[] = {
+ [IEEE80211_RADIOTAP_TSFT] = 0x88,
+ [IEEE80211_RADIOTAP_FLAGS] = 0x11,
+ [IEEE80211_RADIOTAP_RATE] = 0x11,
+ [IEEE80211_RADIOTAP_CHANNEL] = 0x24,
+ [IEEE80211_RADIOTAP_FHSS] = 0x22,
+ [IEEE80211_RADIOTAP_DBM_ANTSIGNAL] = 0x11,
+ [IEEE80211_RADIOTAP_DBM_ANTNOISE] = 0x11,
+ [IEEE80211_RADIOTAP_LOCK_QUALITY] = 0x22,
+ [IEEE80211_RADIOTAP_TX_ATTENUATION] = 0x22,
+ [IEEE80211_RADIOTAP_DB_TX_ATTENUATION] = 0x22,
+ [IEEE80211_RADIOTAP_DBM_TX_POWER] = 0x11,
+ [IEEE80211_RADIOTAP_ANTENNA] = 0x11,
+ [IEEE80211_RADIOTAP_DB_ANTSIGNAL] = 0x11,
+ [IEEE80211_RADIOTAP_DB_ANTNOISE] = 0x11
+ /*
+ * add more here as they are defined in
+ * include/net/ieee80211_radiotap.h
+ */
+ };
+
+ /*
+ * for every radiotap entry we can at
+ * least skip (by knowing the length)...
+ */
+
+ while (iterator->arg_index < sizeof(rt_sizes)) {
+ int hit = 0;
+
+ if (!(iterator->bitmap_shifter & 1))
+ goto next_entry; /* arg not present */
+
+ /*
+ * arg is present, account for alignment padding
+ * 8-bit args can be at any alignment
+ * 16-bit args must start on 16-bit boundary
+ * 32-bit args must start on 32-bit boundary
+ * 64-bit args must start on 64-bit boundary
+ *
+ * note that total arg size can differ from alignment of
+ * elements inside arg, so we use upper nybble of length
+ * table to base alignment on
+ *
+ * also note: these alignments are ** relative to the
+ * start of the radiotap header **. There is no guarantee
+ * that the radiotap header itself is aligned on any
+ * kind of boundary.
+ */
+
+ if ((((int)iterator->arg)-((int)iterator->rtheader)) &
+ ((rt_sizes[iterator->arg_index] >> 4) - 1))
+ iterator->arg_index +=
+ (rt_sizes[iterator->arg_index] >> 4) -
+ ((((int)iterator->arg) -
+ ((int)iterator->rtheader)) &
+ ((rt_sizes[iterator->arg_index] >> 4) - 1));
+
+ /*
+ * this is what we will return to user, but we need to
+ * move on first so next call has something fresh to test
+ */
+
+ iterator->this_arg_index = iterator->arg_index;
+ iterator->this_arg = iterator->arg;
+ hit = 1;
+
+ /* internally move on the size of this arg */
+
+ iterator->arg += rt_sizes[iterator->arg_index] & 0x0f;
+
+ /*
+ * check for insanity where we are given a bitmap that
+ * claims to have more arg content than the length of the
+ * radiotap section. We will normally end up equalling this
+ * max_length on the last arg, never exceeding it.
+ */
+
+ if ((((int)iterator->arg) - ((int)iterator->rtheader)) >
+ iterator->max_length)
+ return -EINVAL;
+
+ next_entry:
+
+ iterator->arg_index++;
+ if (unlikely((iterator->arg_index & 31) == 0)) {
+ /* completed current u32 bitmap */
+ if (iterator->bitmap_shifter & 1) {
+ /* b31 was set, there is more */
+ /* move to next u32 bitmap */
+ iterator->bitmap_shifter = le32_to_cpu(
+ *iterator->next_bitmap);
+ iterator->next_bitmap++;
+ } else {
+ /* no more bitmaps: end */
+ iterator->arg_index = sizeof(rt_sizes);
+ }
+ } else { /* just try the next bit */
+ iterator->bitmap_shifter >>= 1;
+ }
+
+ /* if we found a valid arg earlier, return it now */
+
+ if (hit)
+ return iterator->this_arg_index;
+
+ }
+
+ /* we don't know how to handle any more args, we're done */
+
+ return -1;
+}
+
+EXPORT_SYMBOL(ieee80211_radiotap_iterator_next);
--
Johannes Berg wrote:
Hi Johannes -
>> + * care of alignment handling and extended present fields. interator->this_arg
>
> typo: "interator", also I don't understand why you would want to change
> this_arg?
Thanks for the comments which I addressed in try #9.
iterator->this_arg can make sense to increment when you are walking
through one of the compound arguments, like the channel one, the comment
means you don't have to take care to preserve it during the processing
of any argument in your loop.
I did move the function comments into cfg80211 but I didn't prefer it
and when I read Randy's take I moved them back.
-Andy
Johannes Berg wrote:
> Hi Andy,
>
> Thanks for the repost.
>
>> +/* Radiotap header iteration
>> + * implemented in net/wireless/radiotap.c
>> + *
>> + * call __ieee80211_radiotap_iterator_init() to init a semi-opaque iterator
>> + * struct ieee80211_radiotap_iterator (no need to init the struct beforehand)
>> + * then loop calling __ieee80211_radiotap_iterator_next()... it returns -1
>> + * if there are no more args in the header, or the next argument type index
>> + * that is present. The iterator's this_arg member points to the start of the
>> + * argument associated with the current argument index that is present,
>> + * which can be found in the iterator's this_arg_index member. This arg
>> + * index corresponds to the IEEE80211_RADIOTAP_... defines.
>> + */
>
> I don't think this will show up in the kdoc output, but it sure would be
> nice. Maybe it could be part of the description of
> ieee80211_radiotap_iterator as a 'how to use this iterator' section (see
> kernel-doc-nano-HOWTO.txt, it talks about sections)? Or maybe as
> kernel-doc for the functions themselves.
>
>> +/**
>> + * ieee80211_radiotap_iterator_init - radiotap parser iterator initialization
>
> Oh, you have kerneldoc comments in the source file. Does anyone know
> which is preferable? I usually put everything into the header file that
> is not implementation-detail.
We mostly put kernel-doc into source files (i.e., not header files).
We use kernel-doc in header files for inline functions, macros,
structs, unions, etc., but function kernel-doc blocks are meant to
immediately precede their functions.
--
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
Hi Andy,
Thanks for the repost.
> +/* Radiotap header iteration
> + * implemented in net/wireless/radiotap.c
> + *
> + * call __ieee80211_radiotap_iterator_init() to init a semi-opaque iterator
> + * struct ieee80211_radiotap_iterator (no need to init the struct beforehand)
> + * then loop calling __ieee80211_radiotap_iterator_next()... it returns -1
> + * if there are no more args in the header, or the next argument type index
> + * that is present. The iterator's this_arg member points to the start of the
> + * argument associated with the current argument index that is present,
> + * which can be found in the iterator's this_arg_index member. This arg
> + * index corresponds to the IEEE80211_RADIOTAP_... defines.
> + */
I don't think this will show up in the kdoc output, but it sure would be
nice. Maybe it could be part of the description of
ieee80211_radiotap_iterator as a 'how to use this iterator' section (see
kernel-doc-nano-HOWTO.txt, it talks about sections)? Or maybe as
kernel-doc for the functions themselves.
> +/**
> + * ieee80211_radiotap_iterator_init - radiotap parser iterator initialization
Oh, you have kerneldoc comments in the source file. Does anyone know
which is preferable? I usually put everything into the header file that
is not implementation-detail.
> + * no need to check again for blowing past stated radiotap
> + * header length, becuase ieee80211_radiotap_iterator_next
^^^^^^^
typo
> + * care of alignment handling and extended present fields. interator->this_arg
typo: "interator", also I don't understand why you would want to change
this_arg?
johannes