On Wed, May 12, 2010 at 11:46 AM, Gertjan van Wingerde
<gwingerde@xxxxxxxxx> wrote:
> On Wed, May 12, 2010 at 8:59 AM, Ivo Van Doorn <ivdoorn@xxxxxxxxx> wrote:
>> On Tue, May 11, 2010 at 11:51 PM, Gertjan van Wingerde
>> <gwingerde@xxxxxxxxx> 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@xxxxxxxxx>
>>> ---
>>> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
