[rt2x00-users] [PATCH 2/9] rt2x00: Fix beacon descriptor writing for rt61pci.

Ivo Van Doorn ivdoorn at gmail.com
Thu May 13 03:43:05 AEST 2010


On Wed, May 12, 2010 at 11:46 AM, Gertjan van Wingerde
<gwingerde at gmail.com> wrote:
> On Wed, May 12, 2010 at 8:59 AM, Ivo Van Doorn <ivdoorn at gmail.com> wrote:
>> On Tue, May 11, 2010 at 11:51 PM, Gertjan van Wingerde
>> <gwingerde at gmail.com> wrote:
>>> The buffer address descriptor word is not part of the TXINFO structure
>>> needed for beacons. The current writing of that word for beacons is
>>> therefore an out-of-bounds write.
>>> Fix this by only writing the buffer address descriptor word for TX
>>> queues.
>>>
>>> Signed-off-by: Gertjan van Wingerde <gwingerde at gmail.com>
>>> ---
>>>  drivers/net/wireless/rt2x00/rt61pci.c |   10 +++++-----
>>>  1 files changed, 5 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/net/wireless/rt2x00/rt61pci.c b/drivers/net/wireless/rt2x00/rt61pci.c
>>> index 2436363..99c2981 100644
>>> --- a/drivers/net/wireless/rt2x00/rt61pci.c
>>> +++ b/drivers/net/wireless/rt2x00/rt61pci.c
>>> @@ -1801,12 +1801,12 @@ static void rt61pci_write_tx_desc(struct rt2x00_dev *rt2x00dev,
>>>        rt2x00_set_field32(&word, TXD_W5_WAITING_DMA_DONE_INT, 1);
>>>        rt2x00_desc_write(txd, 5, word);
>>>
>>> -       rt2x00_desc_read(txd, 6, &word);
>>> -       rt2x00_set_field32(&word, TXD_W6_BUFFER_PHYSICAL_ADDRESS,
>>> -                          skbdesc->skb_dma);
>>> -       rt2x00_desc_write(txd, 6, word);
>>> +       if (txdesc->queue != QID_BEACON) {
>>> +               rt2x00_desc_read(txd, 6, &word);
>>> +               rt2x00_set_field32(&word, TXD_W6_BUFFER_PHYSICAL_ADDRESS,
>>> +                                  skbdesc->skb_dma);
>>> +               rt2x00_desc_write(txd, 6, word);
>>>
>>> -       if (skbdesc->desc_len > TXINFO_SIZE) {
>>>                rt2x00_desc_read(txd, 11, &word);
>>>                rt2x00_set_field32(&word, TXD_W11_BUFFER_LENGTH0,
>>>                                   txdesc->length);
>>
>> Shouldn't the check for TXINFO_SIZE be used rather than explicitly
>> checking for the QID?
>>
>
> I agree that this is a change that didn't have to be made in this patch.
> However, after patch 4 of the series we cannot depend on the
> skbdesc->desc_len being set anymore, and we would have to depend on
> checking the QID anyway.
> Note that in reality these two checks are completely equivalent with
> respect to the result.

Hmm, is that a good idea? I mean we are using the skbdesc inside the
function, but we can't be sure that one of the basic values contains the right
value?

Ivo




More information about the users mailing list