All of lore.kernel.org
 help / color / mirror / Atom feed
From: reinette chatre <reinette.chatre@intel.com>
To: "Winkler, Tomas" <tomas.winkler@intel.com>
Cc: "Zhu, Yi" <yi.zhu@intel.com>, Roel Kluin <roel.kluin@gmail.com>,
	"linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>,
	"ipw3945-devel@lists.sourceforge.net"
	<ipw3945-devel@lists.sourceforge.net>,
	Andrew Morton <akpm@linux-foundation.org>
Subject: RE: [PATCH] iwlwifi: Read outside array bounds
Date: Mon, 27 Jul 2009 15:12:15 -0700	[thread overview]
Message-ID: <1248732735.1216.565.camel@rc-desk> (raw)
In-Reply-To: <6F5C1D715B2DA5498A628E6B9C124F040141D0B0DC@hasmsx504.ger.corp.intel.com>

On Mon, 2009-07-27 at 01:28 -0700, Winkler, Tomas wrote:
> 
> > -----Original Message-----
> > From: Zhu, Yi
> > Sent: Monday, July 27, 2009 4:42 AM
> > To: Roel Kluin; Winkler, Tomas; Chatre, Reinette
> > Cc: linux-wireless@vger.kernel.org; ipw3945-devel@lists.sourceforge.net;
> > Andrew Morton
> > Subject: Re: [PATCH] iwlwifi: Read outside array bounds
> > 
> > On Sun, 2009-07-26 at 05:34 +0800, Roel Kluin wrote:
> > > tid is bounded (above) by the size of default_tid_to_tx_fifo (17
> > elements), but
> > > the size of priv->stations[].tid[] is MAX_TID_COUNT (9) elements.
> > 
> > I think MAX_TID_COUNT should be defined as 16 or 17. Tomas?
> >
> 
> In general it's 16. In practice we use only 8.

I think the above statement means that we are mostly using EDCA quality
of service which only uses 8 tids. We do not currently use HCCA (and
thus of course not the hybrid) which would cause more tids to be used.

A closer look at this flow to this function shows:
rs_tl_turn_on_agg 
->rs_tl_turn_on_agg_for_tid
-->ieee80211_start_tx_ba_session
--->iwl_mac_ampdu_action
---->iwl_tx_agg_start

>From what I can tell the tid is not modified from rs_tl_turn_on_agg to
iwl_tx_agg_start and rs_tl_turn_on_agg will not call further with a
value of tid larger than 7 due to its checking.

I thus do not see that tid may be equal or larger than MAX_TID_COUNT at
this point of checking. Even so, having this check will not do harm and
will increase safety.

This patch is already merged and that is ok, I just wanted to add this
information to it.

Reinette




  reply	other threads:[~2009-07-27 22:12 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-07-25 21:34 [PATCH] iwlwifi: Read outside array bounds Roel Kluin
2009-07-27  1:41 ` Zhu Yi
2009-07-27  8:28   ` Winkler, Tomas
2009-07-27 22:12     ` reinette chatre [this message]
2009-07-28  2:27       ` Zhu Yi
2009-07-28  4:50         ` reinette chatre
2009-07-28  5:17           ` Zhu Yi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1248732735.1216.565.camel@rc-desk \
    --to=reinette.chatre@intel.com \
    --cc=akpm@linux-foundation.org \
    --cc=ipw3945-devel@lists.sourceforge.net \
    --cc=linux-wireless@vger.kernel.org \
    --cc=roel.kluin@gmail.com \
    --cc=tomas.winkler@intel.com \
    --cc=yi.zhu@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.