Patchwork drivers/net/wireless/ipw2x00: use strlcpy instead of strncpy

login
register
mail settings
Submitter Chen Gang
Date Jan. 5, 2013, 1:41 p.m.
Message ID <50E82D7D.4090107@asianux.com>
Download mbox | patch
Permalink /patch/209673/
State Not Applicable
Delegated to: David Miller
Headers show

Comments

Chen Gang - Jan. 5, 2013, 1:41 p.m.
The fields must be null-terminated, or IPW_DEBUG_ASSOC will cause issue.

Signed-off-by: Chen Gang <gang.chen@asianux.com>
---
 drivers/net/wireless/ipw2x00/ipw2200.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
Joe Perches - Jan. 5, 2013, 2:42 p.m.
On Sat, 2013-01-05 at 21:41 +0800, Chen Gang wrote:
>   The fields must be null-terminated, or IPW_DEBUG_ASSOC will cause issue.
[]
> diff --git a/drivers/net/wireless/ipw2x00/ipw2200.c b/drivers/net/wireless/ipw2x00/ipw2200.c
[]
> @@ -5558,7 +5558,7 @@ static int ipw_find_adhoc_network(struct ipw_priv *priv,
>  			    min(network->ssid_len, priv->essid_len)))) {
>  			char escaped[IW_ESSID_MAX_SIZE * 2 + 1];
>  
> -			strncpy(escaped,
> +			strlcpy(escaped,
>  				print_ssid(ssid, network->ssid,
>  					   network->ssid_len),
>  				sizeof(escaped));
> @@ -5771,7 +5771,7 @@ static int ipw_best_network(struct ipw_priv *priv,
>  		     memcmp(network->ssid, priv->essid,
>  			    min(network->ssid_len, priv->essid_len)))) {
>  			char escaped[IW_ESSID_MAX_SIZE * 2 + 1];
> -			strncpy(escaped,
> +			strlcpy(escaped,
>  				print_ssid(ssid, network->ssid,
>  					   network->ssid_len),
>  				sizeof(escaped));
> @@ -5788,7 +5788,7 @@ static int ipw_best_network(struct ipw_priv *priv,
>  	 * testing everything else. */
>  	if (match->network && match->network->stats.rssi > network->stats.rssi) {
>  		char escaped[IW_ESSID_MAX_SIZE * 2 + 1];
> -		strncpy(escaped,
> +		strlcpy(escaped,
>  			print_ssid(ssid, network->ssid, network->ssid_len),
>  			sizeof(escaped));
>  		IPW_DEBUG_ASSOC("Network '%s (%pM)' excluded because "

This happens because escaped is declared the wrong size.

It'd be better to change
	char escaped[IW_ESSID_MAX_SIZE * 2 + 1];
to
	DECLARE_SSID_BUF(escaped);
and use
	print_ssid(escaped, network->ssid, network->ssid_len)
in the debug.


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Chen Gang - Jan. 7, 2013, 2:49 a.m.
于 2013年01月05日 22:42, Joe Perches 写道:
> This happens because escaped is declared the wrong size.
> 
> It'd be better to change
> 	char escaped[IW_ESSID_MAX_SIZE * 2 + 1];
> to
> 	DECLARE_SSID_BUF(escaped);
> and use
> 	print_ssid(escaped, network->ssid, network->ssid_len)
> in the debug.
> 

  if what you said is true:
    it is better to delete escaped variable
    use ssid instead of escaped, directly.

  but I think the original author intended to use escaped instead of ssid
    DECLARE_SSID_BUF(ssid)  (line 5525, 5737)
    use ssid to print debug information directly
      (such as: line 5530..5535, 5545..5549, 5745..5749, ...)
    when need print additional information, use escaped
      (line 5559..5569, 5773..5782, 5791..5799)

  so, I still suggest:
    only fix the bug (use strlcpy instead of strncpy)
    and not touch original features which orignal author intended using.

  Regards

gchen.

in drivers/net/wireless/ipw2x00/ipw2200.c:

 5519 static int ipw_find_adhoc_network(struct ipw_priv *priv,
 5520                                   struct ipw_network_match *match,
 5521                                   struct libipw_network *network,
 5522                                   int roaming)
 5523 {
 5524         struct ipw_supported_rates rates;
 5525         DECLARE_SSID_BUF(ssid);
 5526 
 5527         /* Verify that this network's capability is compatible with the
 5528          * current mode (AdHoc or Infrastructure) */
 5529         if ((priv->ieee->iw_mode == IW_MODE_ADHOC &&
 5530              !(network->capability & WLAN_CAPABILITY_IBSS))) {
 5531                 IPW_DEBUG_MERGE("Network '%s (%pM)' excluded due to "
 5532                                 "capability mismatch.\n",
 5533                                 print_ssid(ssid, network->ssid,
 5534                                            network->ssid_len),
 5535                                 network->bssid);
 5536                 return 0;
 5537         }
 5538 
 5539         if (unlikely(roaming)) {
 5540                 /* If we are roaming, then ensure check if this is a valid
 5541                  * network to try and roam to */
 5542                 if ((network->ssid_len != match->network->ssid_len) ||
 5543                     memcmp(network->ssid, match->network->ssid,
 5544                            network->ssid_len)) {
 5545                         IPW_DEBUG_MERGE("Network '%s (%pM)' excluded "
 5546                                         "because of non-network ESSID.\n",
 5547                                         print_ssid(ssid, network->ssid,
 5548                                                    network->ssid_len),
 5549                                         network->bssid);
 5550                         return 0;
 5551                 }
 5552         } else {
 5553                 /* If an ESSID has been configured then compare the broadcast
 5554                  * ESSID to ours */
 5555                 if ((priv->config & CFG_STATIC_ESSID) &&
 5556                     ((network->ssid_len != priv->essid_len) ||
 5557                      memcmp(network->ssid, priv->essid,
 5558                             min(network->ssid_len, priv->essid_len)))) {
 5559                         char escaped[IW_ESSID_MAX_SIZE * 2 + 1];
 5560 
 5561                         strncpy(escaped,
 5562                                 print_ssid(ssid, network->ssid,
 5563                                            network->ssid_len),
 5564                                 sizeof(escaped));
 5565                         IPW_DEBUG_MERGE("Network '%s (%pM)' excluded "
 5566                                         "because of ESSID mismatch: '%s'.\n",
 5567                                         escaped, network->bssid,
 5568                                         print_ssid(ssid, priv->essid,
 5569                                                    priv->essid_len));
 5570                         return 0;
 5571                 }
 5572         }
 ...
 
 5732 static int ipw_best_network(struct ipw_priv *priv,
 5733                             struct ipw_network_match *match,
 5734                             struct libipw_network *network, int roaming)
 5735 {
 5736         struct ipw_supported_rates rates;
 5737         DECLARE_SSID_BUF(ssid);
 5738 
 5739         /* Verify that this network's capability is compatible with the
 5740          * current mode (AdHoc or Infrastructure) */
 5741         if ((priv->ieee->iw_mode == IW_MODE_INFRA &&
 5742              !(network->capability & WLAN_CAPABILITY_ESS)) ||
 5743             (priv->ieee->iw_mode == IW_MODE_ADHOC &&
 5744              !(network->capability & WLAN_CAPABILITY_IBSS))) {
 5745                 IPW_DEBUG_ASSOC("Network '%s (%pM)' excluded due to "
 5746                                 "capability mismatch.\n",
 5747                                 print_ssid(ssid, network->ssid,
 5748                                            network->ssid_len),
 5749                                 network->bssid);
 5750                 return 0;
 5751         }
 5752 
 5753         if (unlikely(roaming)) {
 5754                 /* If we are roaming, then ensure check if this is a valid
 5755                  * network to try and roam to */
 5756                 if ((network->ssid_len != match->network->ssid_len) ||
 5757                     memcmp(network->ssid, match->network->ssid,
 5758                            network->ssid_len)) {
 5759                         IPW_DEBUG_ASSOC("Network '%s (%pM)' excluded "
 5760                                         "because of non-network ESSID.\n",
 5761                                         print_ssid(ssid, network->ssid,
 5762                                                    network->ssid_len),
 5763                                         network->bssid);
 5764                         return 0;
 5765                 }
 5766         } else {
 5767                 /* If an ESSID has been configured then compare the broadcast
 5768                  * ESSID to ours */
 5769                 if ((priv->config & CFG_STATIC_ESSID) &&
 5770                     ((network->ssid_len != priv->essid_len) ||
 5771                      memcmp(network->ssid, priv->essid,
 5772                             min(network->ssid_len, priv->essid_len)))) {
 5773                         char escaped[IW_ESSID_MAX_SIZE * 2 + 1];
 5774                         strncpy(escaped,
 5775                                 print_ssid(ssid, network->ssid,
 5776                                            network->ssid_len),
 5777                                 sizeof(escaped));
 5778                         IPW_DEBUG_ASSOC("Network '%s (%pM)' excluded "
 5779                                         "because of ESSID mismatch: '%s'.\n",
 5780                                         escaped, network->bssid,
 5781                                         print_ssid(ssid, priv->essid,
 5782                                                    priv->essid_len));
 5783                         return 0;
 5784                 }
 5785         }
 5786 
 5787         /* If the old network rate is better than this one, don't bother
 5788          * testing everything else. */
 5789         if (match->network && match->network->stats.rssi > network->stats.rssi) {
 5790                 char escaped[IW_ESSID_MAX_SIZE * 2 + 1];
 5791                 strncpy(escaped,
 5792                         print_ssid(ssid, network->ssid, network->ssid_len),
 5793                         sizeof(escaped));
 5794                 IPW_DEBUG_ASSOC("Network '%s (%pM)' excluded because "
 5795                                 "'%s (%pM)' has a stronger signal.\n",
 5796                                 escaped, network->bssid,
 5797                                 print_ssid(ssid, match->network->ssid,
 5798                                            match->network->ssid_len),
 5799                                 match->network->bssid);
 5800                 return 0;
 5801         }
Chen Gang F T - Jan. 7, 2013, 2:57 a.m.
于 2013年01月07日 10:49, Chen Gang 写道:
> 于 2013年01月05日 22:42, Joe Perches 写道:
>> This happens because escaped is declared the wrong size.
>>
>> It'd be better to change
>> 	char escaped[IW_ESSID_MAX_SIZE * 2 + 1];
>> to
>> 	DECLARE_SSID_BUF(escaped);
>> and use
>> 	print_ssid(escaped, network->ssid, network->ssid_len)
>> in the debug.
>>
> 
>   if what you said is true:
>     it is better to delete escaped variable
>     use ssid instead of escaped, directly.
> 
   oh, sorry, it is my fault.
   we need use duplicate buffer to print different contents, at the same time.

   :-)

   but I still suggest to keep original author using
      maybe he intend to keep the print size for output format
      so I think it is better to only fix bug, not touch the features. 

   Regards

 gchen.



>   but I think the original author intended to use escaped instead of ssid
>     DECLARE_SSID_BUF(ssid)  (line 5525, 5737)
>     use ssid to print debug information directly
>       (such as: line 5530..5535, 5545..5549, 5745..5749, ...)
>     when need print additional information, use escaped
>       (line 5559..5569, 5773..5782, 5791..5799)
> 
>   so, I still suggest:
>     only fix the bug (use strlcpy instead of strncpy)
>     and not touch original features which orignal author intended using.
> 
>   Regards
> 
> gchen.
> 
> in drivers/net/wireless/ipw2x00/ipw2200.c:
> 
>  5519 static int ipw_find_adhoc_network(struct ipw_priv *priv,
>  5520                                   struct ipw_network_match *match,
>  5521                                   struct libipw_network *network,
>  5522                                   int roaming)
>  5523 {
>  5524         struct ipw_supported_rates rates;
>  5525         DECLARE_SSID_BUF(ssid);
>  5526 
>  5527         /* Verify that this network's capability is compatible with the
>  5528          * current mode (AdHoc or Infrastructure) */
>  5529         if ((priv->ieee->iw_mode == IW_MODE_ADHOC &&
>  5530              !(network->capability & WLAN_CAPABILITY_IBSS))) {
>  5531                 IPW_DEBUG_MERGE("Network '%s (%pM)' excluded due to "
>  5532                                 "capability mismatch.\n",
>  5533                                 print_ssid(ssid, network->ssid,
>  5534                                            network->ssid_len),
>  5535                                 network->bssid);
>  5536                 return 0;
>  5537         }
>  5538 
>  5539         if (unlikely(roaming)) {
>  5540                 /* If we are roaming, then ensure check if this is a valid
>  5541                  * network to try and roam to */
>  5542                 if ((network->ssid_len != match->network->ssid_len) ||
>  5543                     memcmp(network->ssid, match->network->ssid,
>  5544                            network->ssid_len)) {
>  5545                         IPW_DEBUG_MERGE("Network '%s (%pM)' excluded "
>  5546                                         "because of non-network ESSID.\n",
>  5547                                         print_ssid(ssid, network->ssid,
>  5548                                                    network->ssid_len),
>  5549                                         network->bssid);
>  5550                         return 0;
>  5551                 }
>  5552         } else {
>  5553                 /* If an ESSID has been configured then compare the broadcast
>  5554                  * ESSID to ours */
>  5555                 if ((priv->config & CFG_STATIC_ESSID) &&
>  5556                     ((network->ssid_len != priv->essid_len) ||
>  5557                      memcmp(network->ssid, priv->essid,
>  5558                             min(network->ssid_len, priv->essid_len)))) {
>  5559                         char escaped[IW_ESSID_MAX_SIZE * 2 + 1];
>  5560 
>  5561                         strncpy(escaped,
>  5562                                 print_ssid(ssid, network->ssid,
>  5563                                            network->ssid_len),
>  5564                                 sizeof(escaped));
>  5565                         IPW_DEBUG_MERGE("Network '%s (%pM)' excluded "
>  5566                                         "because of ESSID mismatch: '%s'.\n",
>  5567                                         escaped, network->bssid,
>  5568                                         print_ssid(ssid, priv->essid,
>  5569                                                    priv->essid_len));
>  5570                         return 0;
>  5571                 }
>  5572         }
>  ...
>  
>  5732 static int ipw_best_network(struct ipw_priv *priv,
>  5733                             struct ipw_network_match *match,
>  5734                             struct libipw_network *network, int roaming)
>  5735 {
>  5736         struct ipw_supported_rates rates;
>  5737         DECLARE_SSID_BUF(ssid);
>  5738 
>  5739         /* Verify that this network's capability is compatible with the
>  5740          * current mode (AdHoc or Infrastructure) */
>  5741         if ((priv->ieee->iw_mode == IW_MODE_INFRA &&
>  5742              !(network->capability & WLAN_CAPABILITY_ESS)) ||
>  5743             (priv->ieee->iw_mode == IW_MODE_ADHOC &&
>  5744              !(network->capability & WLAN_CAPABILITY_IBSS))) {
>  5745                 IPW_DEBUG_ASSOC("Network '%s (%pM)' excluded due to "
>  5746                                 "capability mismatch.\n",
>  5747                                 print_ssid(ssid, network->ssid,
>  5748                                            network->ssid_len),
>  5749                                 network->bssid);
>  5750                 return 0;
>  5751         }
>  5752 
>  5753         if (unlikely(roaming)) {
>  5754                 /* If we are roaming, then ensure check if this is a valid
>  5755                  * network to try and roam to */
>  5756                 if ((network->ssid_len != match->network->ssid_len) ||
>  5757                     memcmp(network->ssid, match->network->ssid,
>  5758                            network->ssid_len)) {
>  5759                         IPW_DEBUG_ASSOC("Network '%s (%pM)' excluded "
>  5760                                         "because of non-network ESSID.\n",
>  5761                                         print_ssid(ssid, network->ssid,
>  5762                                                    network->ssid_len),
>  5763                                         network->bssid);
>  5764                         return 0;
>  5765                 }
>  5766         } else {
>  5767                 /* If an ESSID has been configured then compare the broadcast
>  5768                  * ESSID to ours */
>  5769                 if ((priv->config & CFG_STATIC_ESSID) &&
>  5770                     ((network->ssid_len != priv->essid_len) ||
>  5771                      memcmp(network->ssid, priv->essid,
>  5772                             min(network->ssid_len, priv->essid_len)))) {
>  5773                         char escaped[IW_ESSID_MAX_SIZE * 2 + 1];
>  5774                         strncpy(escaped,
>  5775                                 print_ssid(ssid, network->ssid,
>  5776                                            network->ssid_len),
>  5777                                 sizeof(escaped));
>  5778                         IPW_DEBUG_ASSOC("Network '%s (%pM)' excluded "
>  5779                                         "because of ESSID mismatch: '%s'.\n",
>  5780                                         escaped, network->bssid,
>  5781                                         print_ssid(ssid, priv->essid,
>  5782                                                    priv->essid_len));
>  5783                         return 0;
>  5784                 }
>  5785         }
>  5786 
>  5787         /* If the old network rate is better than this one, don't bother
>  5788          * testing everything else. */
>  5789         if (match->network && match->network->stats.rssi > network->stats.rssi) {
>  5790                 char escaped[IW_ESSID_MAX_SIZE * 2 + 1];
>  5791                 strncpy(escaped,
>  5792                         print_ssid(ssid, network->ssid, network->ssid_len),
>  5793                         sizeof(escaped));
>  5794                 IPW_DEBUG_ASSOC("Network '%s (%pM)' excluded because "
>  5795                                 "'%s (%pM)' has a stronger signal.\n",
>  5796                                 escaped, network->bssid,
>  5797                                 print_ssid(ssid, match->network->ssid,
>  5798                                            match->network->ssid_len),
>  5799                                 match->network->bssid);
>  5800                 return 0;
>  5801         }
>

Patch

diff --git a/drivers/net/wireless/ipw2x00/ipw2200.c b/drivers/net/wireless/ipw2x00/ipw2200.c
index 844f201..c85261b 100644
--- a/drivers/net/wireless/ipw2x00/ipw2200.c
+++ b/drivers/net/wireless/ipw2x00/ipw2200.c
@@ -5558,7 +5558,7 @@  static int ipw_find_adhoc_network(struct ipw_priv *priv,
 			    min(network->ssid_len, priv->essid_len)))) {
 			char escaped[IW_ESSID_MAX_SIZE * 2 + 1];
 
-			strncpy(escaped,
+			strlcpy(escaped,
 				print_ssid(ssid, network->ssid,
 					   network->ssid_len),
 				sizeof(escaped));
@@ -5771,7 +5771,7 @@  static int ipw_best_network(struct ipw_priv *priv,
 		     memcmp(network->ssid, priv->essid,
 			    min(network->ssid_len, priv->essid_len)))) {
 			char escaped[IW_ESSID_MAX_SIZE * 2 + 1];
-			strncpy(escaped,
+			strlcpy(escaped,
 				print_ssid(ssid, network->ssid,
 					   network->ssid_len),
 				sizeof(escaped));
@@ -5788,7 +5788,7 @@  static int ipw_best_network(struct ipw_priv *priv,
 	 * testing everything else. */
 	if (match->network && match->network->stats.rssi > network->stats.rssi) {
 		char escaped[IW_ESSID_MAX_SIZE * 2 + 1];
-		strncpy(escaped,
+		strlcpy(escaped,
 			print_ssid(ssid, network->ssid, network->ssid_len),
 			sizeof(escaped));
 		IPW_DEBUG_ASSOC("Network '%s (%pM)' excluded because "