mysterious -1 offset in staging rt2860 driver

Dan Carpenter

author

2010-03-04 19:14 UTC #0 lore raw

So here is a mystery for people who enjoy such mysterious things.

drivers/staging/rt2860/sta_ioctl.c +1020 rt_ioctl_giwscan(219)
1011 if (CAP_IS_PRIVACY_ON
1012 (pAdapter->ScanTab.BssEntry[i].CapabilityInfo))
1013 iwe.u.data.flags = IW_ENCODE_ENABLED | IW_ENCODE_NOKEY;
1014 else
1015 iwe.u.data.flags = IW_ENCODE_DISABLED;
1016

  1017                  __smatch_value("iwe.u.data.flags");
  1018                  previous_ev = current_ev;
  1019                  current_ev   1020                      iwe_stream_add_point(info, current_ev, end_buf, &iwe,

1021 (char *)pAdapter->
1022 SharedKey[BSS0][(iwe.u.data.
1023 flags &
1024 IW_ENCODE_INDEX) -
1025 1].Key);

That last bit is easier to read if it's on one line.

SharedKey[BSS0][(iwe.u.data.flags & IW_ENCODE_INDEX) - 1].Key);

iwe.u.data.flags is either 0x0400 or 0x8000. IW_ENCODE_INDEX is 0xff, so after we mask off the lower bits we get 0x00 and then we subtract 1 we get an array offset of -1.

I doubt that's what we want, but what _do_ we want here?

regards,
dan carpenter

walter harms

2010-03-05 09:44 UTC #1 lore raw

Quoted 21 lines · Dan Carpenter

So here is a mystery for people who enjoy such mysterious things.
drivers/staging/rt2860/sta_ioctl.c +1020 rt_ioctl_giwscan(219)
1011 if (CAP_IS_PRIVACY_ON
1012 (pAdapter->ScanTab.BssEntry[i].CapabilityInfo))
1013 iwe.u.data.flags = IW_ENCODE_ENABLED | IW_ENCODE_NOKEY;
1014 else
1015 iwe.u.data.flags = IW_ENCODE_DISABLED;
1016
  1017                  __smatch_value("iwe.u.data.flags");
  1018                  previous_ev = current_ev;
  1019                  current_ev >   1020                      iwe_stream_add_point(info, current_ev, end_buf, &iwe,
1021 (char *)pAdapter->
1022 SharedKey[BSS0][(iwe.u.data.
1023 flags &
1024 IW_ENCODE_INDEX) -
1025 1].Key);
That last bit is easier to read if it's on one line.

Quoted 7 lines

SharedKey[BSS0][(iwe.u.data.flags & IW_ENCODE_INDEX) - 1].Key);
iwe.u.data.flags is either 0x0400 or 0x8000. IW_ENCODE_INDEX is 0xff, so after we mask off the lower bits we get 0x00 and then we subtract 1 we get an array offset of -1.
I doubt that's what we want, but what _do_ we want here?

I used http://tomoyo.sourceforge.jp/cgi-bin/lxr/ident?i=IW_ENCODE_INDEX to get an idea, and it seems most times the index is protected by
if ( idx<0) what seems to indicate that the index should start with 1 (very wired). NTL this is unreadable code and should be desected.
perhaps:

idx=iwe.u.data.flags & IW_ENCODE_INDEX) - 1
char *key=pAdapter->SharedKey[BSS0][idx].key

but i have still no idea about this -1 it does not fit well.

just my 2 cents,
walter

Quoted 8 lines

regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Darren Jenkins

2010-03-06 00:21 UTC #2 lore raw

In reply to Dan Carpenter

Quoted 21 lines · Dan Carpenter

So here is a mystery for people who enjoy such mysterious things.
drivers/staging/rt2860/sta_ioctl.c +1020 rt_ioctl_giwscan(219)
1011 if (CAP_IS_PRIVACY_ON
1012 (pAdapter->ScanTab.BssEntry[i].CapabilityInfo))
1013 iwe.u.data.flags = IW_ENCODE_ENABLED | IW_ENCODE_NOKEY;
1014 else
1015 iwe.u.data.flags = IW_ENCODE_DISABLED;
1016
1017 __smatch_value("iwe.u.data.flags");
1018 previous_ev = current_ev;
1019 current_ev > 1020 iwe_stream_add_point(info, current_ev, end_buf, &iwe, 1021 (char *)pAdapter->
1022 SharedKey[BSS0][(iwe.u.data.
1023 flags &
1024 IW_ENCODE_INDEX) -
1025 1].Key);
That last bit is easier to read if it's on one line.

Quoted 8 lines

SharedKey[BSS0][(iwe.u.data.flags & IW_ENCODE_INDEX) - 1].Key);
iwe.u.data.flags is either 0x0400 or 0x8000. IW_ENCODE_INDEX is 0xff, so after we mask off the lower bits we get 0x00 and then we subtract 1 we get an array offset of -1.
I doubt that's what we want, but what _do_ we want here?

Looking at the use of IW_ENCODE_INDEX elsewhere it seems to be a mask for a key value that can be 0..4 where 0 means some default value and 1 - 4 means use value-- ie 0 - 3.

iwe.u.data.flags is assigned to 1 earlier so I would assume the code should be

iwe.u.data.flags &= ~ IW_ENCODE_FLAGS

Quoted 5 lines

  1011                  if (CAP_IS_PRIVACY_ON
  1012                      (pAdapter->ScanTab.BssEntry[i].CapabilityInfo))
  1013                          iwe.u.data.flags |= IW_ENCODE_ENABLED | IW_ENCODE_NOKEY;
  1014                  else
  1015                          iwe.u.data.flags |= IW_ENCODE_DISABLED;

Which I think would both make sense and give an array index of 0.

Darren J.
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html