Re: [ath9k-devel] [v2] ath9k: simplify the code-paths when not using the built-in EEPROM

2016-07-19 Thread Kalle Valo
Martin Blumenstingl  wrote:
> There were two paths in the code for "external" eeprom sources. The code
> in eeprom.c only handled the cases where the eeprom data was loaded via
> request_firmware. ahb.c and pci.c on the other hand had some duplicate
> code which was only used when the eeprom data was passed via
> ath9k_platform_data.
> With this change all eeprom data handling is now unified in eeprom.c.
> 
> Signed-off-by: Martin Blumenstingl 

Thanks, 1 patch applied to ath-next branch of ath.git:

563985199693 ath9k: simplify the code-paths when not using the built-in EEPROM

-- 
Sent by pwcli
https://patchwork.kernel.org/patch/9222187/

___
ath9k-devel mailing list
ath9k-devel@lists.ath9k.org
https://lists.ath9k.org/mailman/listinfo/ath9k-devel


Re: [ath9k-devel] AuthSAE rekeying issues with ATH9K

2016-07-19 Thread Stam, Michel [FINT]
Hey Adrian,

Apologies, I was out of the office on Friday, so I did not have access to my 
development system nor my test units.

It is on the top of my todo list, will get to it hopefully today, or tomorrow.

Cheers,

Michel
--- Begin Message ---
hi,

any news?


-a


On 14 July 2016 at 13:10, Adrian Chadd  wrote:
> Hi,
>
> Yeah, delete and re-add should be fine. Just look at whatever the
> locking requirements are so you're serialised with anything else
> touching the keycache hardware at the same time.
>
> Hopefully it's not such a terrible bug that you end up doing it like,
> hundreds of times a second. :)
>
>
>
> -adrian

--- End Message ---
___
ath9k-devel mailing list
ath9k-devel@lists.ath9k.org
https://lists.ath9k.org/mailman/listinfo/ath9k-devel


Re: [ath9k-devel] AuthSAE rekeying issues with ATH9K

2016-07-19 Thread Adrian Chadd
Hi,

So, there's a keycache index in the RX packet telling you which
keycache entry it matched.

See if it's matching a keycache entry but then failing to decrypt; if
this is happening then an easy check would be "hit some threshold for
crypto failures" => "replumb that keycache slot."
This means ath9k would have to keep copies itself of all keycache
entries, which isn't a /bad/ thing..



-adrian
___
ath9k-devel mailing list
ath9k-devel@lists.ath9k.org
https://lists.ath9k.org/mailman/listinfo/ath9k-devel


Re: [ath9k-devel] AuthSAE rekeying issues with ATH9K

2016-07-19 Thread Adrian Chadd
Hi,

Yeah, delete and re-add should be fine. Just look at whatever the
locking requirements are so you're serialised with anything else
touching the keycache hardware at the same time.

Hopefully it's not such a terrible bug that you end up doing it like,
hundreds of times a second. :)



-adrian
___
ath9k-devel mailing list
ath9k-devel@lists.ath9k.org
https://lists.ath9k.org/mailman/listinfo/ath9k-devel


Re: [ath9k-devel] AuthSAE rekeying issues with ATH9K

2016-07-19 Thread Adrian Chadd
hi,

any news?


-a


On 14 July 2016 at 13:10, Adrian Chadd  wrote:
> Hi,
>
> Yeah, delete and re-add should be fine. Just look at whatever the
> locking requirements are so you're serialised with anything else
> touching the keycache hardware at the same time.
>
> Hopefully it's not such a terrible bug that you end up doing it like,
> hundreds of times a second. :)
>
>
>
> -adrian
___
ath9k-devel mailing list
ath9k-devel@lists.ath9k.org
https://lists.ath9k.org/mailman/listinfo/ath9k-devel


Re: [ath9k-devel] [PATCH v4 1/3] Documentation: dt: net: add ath9k wireless device binding

2016-07-19 Thread Arnd Bergmann
On Monday, July 11, 2016 11:21:26 PM CEST Martin Blumenstingl wrote:
> On Mon, Jul 11, 2016 at 12:01 AM, Arnd Bergmann  wrote:
> >> ath9k reads the data from the EEPROM into memory. With that property
> >> disabled ath9k simply assumes that the endianness of the values in the
> >> EEPROM are having the correct endianness for the host system (in other
> >> words: no swap is being applied).
> >> I am not sure I understand you correctly, but isn't what you are
> >> explaining an issue in the ath9k code, rather than in this
> >> documentation?
> >
> > I looked at the code more to find that out now, but I'm more confused
> > now, as the eeprom seems to be read as a byte stream, and the endianess
> > conversion that the driver performs is not on the data values in it,
> > but seems to instead swap the bytes in each 16-bit word, regardless
> > of the contents (the values inside of the byte stream are always
> > interpreted as big-endian). Is that a correct observation?
> that seems to be the case for the ar9003 eeprom. Other implementations
> are doing it different, look at ath9k_hw_ar9287_check_eeprom for
> example: first ath9k_hw_nvram_swap_data checks the two magic bytes at
> the beginning of the data and swaps the bytes in each 16-bit word if
> the magic bytes don't match the magic bytes for the "native system
> endianness" (see AR5416_EEPROM_MAGIC). Then more swapping is applied.
> I asked for more details about the EEPROM format (specifically the
> endianness part) here [0] as I don't have access to the datasheets
> (all I have is the ath9k code)

Ok.

> > What I see in ath_pci_eeprom_read() is that the 16-bit words are taken
> > from the lower 16 bit of the little-endian AR_EEPROM_STATUS_DATA
> > register. As this is coming from a PCI register, it must have a device
> > specific endianess that is identical on all CPUs, so in the description
> > above, mentioning CPU endianess is a bit confusing. I could not find
> > the code that does the conditional byteswap, instead this function
> >
> > static bool ar9300_eeprom_read_byte(struct ath_hw *ah, int address,
> > u8 *buffer)
> > {
> > u16 val;
> >
> > if (unlikely(!ath9k_hw_nvram_read(ah, address / 2, )))
> > return false;
> >
> > *buffer = (val >> (8 * (address % 2))) & 0xff;
> > return true;
> > }
> >
> > evidently assumes that the lower 8 bit of the 16-bit data from PCI
> > come first, i.e. it byteswaps on big-endian CPUs to get the bytestream
> > back into the order in which it is stored in the EEPROM.
> Please have a look at the ath9k_hw_nvram_swap_data function and
> eeprom_ops.check_eeprom (for example ath9k_hw_ar9287_check_eeprom).
> These are performing the conditional swapping (in addition to whatever
> previous swapping there was).
> The basic code works like this: read the full EEPROM data into memory
> (either from PCI bus, ath9k_platform_data or request_firmware), then
> eeprom_ops.check_eeprom will call ath9k_hw_nvram_swap_data for 16-bit
> word swapping and afterwards the check_eeprom implementation will doe
> further swapping.
> Apart from that your findings seem correct (at least this is identical
> to how I would interpret the code).

Ok, so my interpretation of what this is done for is that the
swap in ath9k_hw_nvram_swap_data() is done to compensate for
the data that is read byte-reversed from the PCI bus and it
is does not swap when the data is read from a file. The result
is a structure with big-endian 16-bit and 32-bit members but
all fields in the right place.

The swapping in ath9k_hw_ar9287_check_eeprom() then turns the
big-endian fields into little-endian fields so it can be used
on little-endian CPUs without going through le16_to_cpu().

However, the whole thing still looks fragile to me as it
doesn't seem to handle the case where we want to swap the
values but not the bus.

My guess is that we still want to fix the driver to handle
this more consistently in order to decide whether a DT property
is needed or not.

> > Interestingly, this also seems to happen for ath_ahb_eeprom_read()
> > even though on that one the value does not get swapped by the bus
> > accessor, so presumably big-endian machines with a ahb based ath9k
> > store their eeprom byte-reversed?
> on AHB the eeprom data has to be provided via ath9k_platform_data /
> request_firmware mechanism. Thus there is no bus specific swapping,
> only the ath9k_hw_nvram_swap_data / eeprom_ops.check_eeprom swapping
> is applied in this case.

I guess the header then indicates that none of the swapping is
performed.

Arnd
___
ath9k-devel mailing list
ath9k-devel@lists.ath9k.org
https://lists.ath9k.org/mailman/listinfo/ath9k-devel


Re: [ath9k-devel] AuthSAE rekeying issues with ATH9K

2016-07-19 Thread Stam, Michel [FINT]
Hey Adrian,

Just saw your email, I took a quick look and I was able to find a struct 
ieee80211_key_conf which contains the key material.
I suppose that the ath_node needs to be extracted from the ieee80211_sta -> 
drv_priv by looking up the source mac address in the struct ieee80211_hdr 
somewhere in ath9k_rx_accept()?
The ath_node could then give the ps_key which contains the key cache index. 
This function also houses decrypt_errors which could be used for detection.

I suppose that ieee80211_find_sta_by_ifaddr( ) with the destination and source 
mac as arguments would nicely fit the bill.
The struct ath_node could also contain the key material, which could then be 
used for the replumbing.

Any particular pattern I must follow when writing the key material? Or will 
ath_key_delete( ) followed by a ath_hw_set_keycache_entry( ) suffice?

Kind regards,

Michel Stam
--- Begin Message ---
Hi,

So, there's a keycache index in the RX packet telling you which
keycache entry it matched.

See if it's matching a keycache entry but then failing to decrypt; if
this is happening then an easy check would be "hit some threshold for
crypto failures" => "replumb that keycache slot."
This means ath9k would have to keep copies itself of all keycache
entries, which isn't a /bad/ thing..



-adrian

--- End Message ---
___
ath9k-devel mailing list
ath9k-devel@lists.ath9k.org
https://lists.ath9k.org/mailman/listinfo/ath9k-devel