Begin forwarded message:
Date: Sat, 22 Dec 2012 16:28:47 +0000 (UTC)
From: [email protected]
To: [email protected]
Subject: [Bug 51901] New: mac80211/cfg.c:1995: possible bad call to memcpy ?
https://bugzilla.kernel.org/show_bug.cgi?id=51901
Summary: mac80211/cfg.c:1995: possible bad call to memcpy ?
Product: Networking
Version: 2.5
Kernel Version: 3.8-rc1
Platform: All
OS/Version: Linux
Tree: Mainline
Status: NEW
Severity: normal
Priority: P1
Component: Other
AssignedTo: [email protected]
ReportedBy: [email protected]
Regression: Yes
The source code is
static int ieee80211_set_mcast_rate(struct wiphy *wiphy, struct net_device
*dev,
int rate[IEEE80211_NUM_BANDS])
{
struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
memcpy(sdata->vif.bss_conf.mcast_rate, rate, sizeof(rate));
The compiler says
cfg.c: In function ‘int ieee80211_set_mcast_rate(wiphy*, net_device*, int*)’:
cfg.c:1995: warning: argument to ‘sizeof’ in ‘void* memcpy(void*, const void*,
size_t)’ call is the same expression as the source; did you mean to dereference
it? [-Wsizeof-pointer-memaccess]
memcpy(rate2, rate, sizeof(rate));
^
Here is static analyser cppcheck also finding the same problem
[linux-3.8-rc1/net/mac80211/cfg.c:1995]: (error) Using 'sizeof' on array given
as function argument returns size of a pointer.
Suggest code rework.
--
Configure bugmail: https://bugzilla.kernel.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
On 2012-12-27 8:45 AM, Johannes Berg wrote:
> On Sat, 2012-12-22 at 13:55 -0800, Stephen Hemminger wrote:
>
>> static int ieee80211_set_mcast_rate(struct wiphy *wiphy, struct net_device
>> *dev,
>> int rate[IEEE80211_NUM_BANDS])
>> {
>> struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
>>
>> memcpy(sdata->vif.bss_conf.mcast_rate, rate, sizeof(rate));
>
> Seems fine to me. If anyone's bothered by the warning, patch welcome,
> but the code is ok since it's an array.
I think the warning is correct and the code needs to be fixed. Since
rate is a function parameter, sizeof(rate) == sizeof(int).
I didn't know about this weird aspect of the C standard either, but I
verified it with a simple user space test program. ;)
- Felix
On Thu, 2012-12-27 at 09:30 +0100, Felix Fietkau wrote:
> On 2012-12-27 8:45 AM, Johannes Berg wrote:
> > On Sat, 2012-12-22 at 13:55 -0800, Stephen Hemminger wrote:
> >
> >> static int ieee80211_set_mcast_rate(struct wiphy *wiphy, struct net_device
> >> *dev,
> >> int rate[IEEE80211_NUM_BANDS])
> >> {
> >> struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
> >>
> >> memcpy(sdata->vif.bss_conf.mcast_rate, rate, sizeof(rate));
> >
> > Seems fine to me. If anyone's bothered by the warning, patch welcome,
> > but the code is ok since it's an array.
> I think the warning is correct and the code needs to be fixed. Since
> rate is a function parameter, sizeof(rate) == sizeof(int).
> I didn't know about this weird aspect of the C standard either, but I
> verified it with a simple user space test program. ;)
Yeah you're right, I even tried to test it but got it wrong. It should
be changed.
johannes
On Sat, 2012-12-22 at 13:55 -0800, Stephen Hemminger wrote:
> static int ieee80211_set_mcast_rate(struct wiphy *wiphy, struct net_device
> *dev,
> int rate[IEEE80211_NUM_BANDS])
> {
> struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
>
> memcpy(sdata->vif.bss_conf.mcast_rate, rate, sizeof(rate));
Seems fine to me. If anyone's bothered by the warning, patch welcome,
but the code is ok since it's an array.
johannes