This was blocking sending SMPS action frames
through debugfs.
Signed-off-by: Chaitanya T K <[email protected]>
---
net/mac80211/debugfs_netdev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/mac80211/debugfs_netdev.c b/net/mac80211/debugfs_netdev.c
index 04b5a14..1a8fa5f 100644
--- a/net/mac80211/debugfs_netdev.c
+++ b/net/mac80211/debugfs_netdev.c
@@ -265,7 +265,7 @@ static ssize_t ieee80211_if_parse_smps(struct ieee80211_sub_if_data *sdata,
enum ieee80211_smps_mode mode;
for (mode = 0; mode < IEEE80211_SMPS_NUM_MODES; mode++) {
- if (strncmp(buf, smps_modes[mode], buflen) == 0) {
+ if (strncmp(buf, smps_modes[mode], buflen-1) == 0) {
int err = ieee80211_set_smps(sdata, mode);
if (!err)
return buflen;
On Mon, Jan 6, 2014 at 8:18 PM, Johannes Berg <[email protected]> wrote:
> On Sun, 2014-01-05 at 02:36 +0530, Chaitanya T K wrote:
>> This was blocking sending SMPS action frames
>> through debugfs.
>
> I don't see any issue here, explain.
>
> johannes
>
buflen includes the new line character as well, hence the comparison
strncmp fails for all combiantions.
echo "static" > ieee80211/phyX/netdev\:wlanX/smps
Then
buf=static\n
buflen=7
But the comparison is with "static" which doesn't include "\n"
hence the comparison fails.
On Mon, 2014-01-06 at 20:35 +0530, Krishna Chaitanya wrote:
> On Mon, Jan 6, 2014 at 8:18 PM, Johannes Berg <[email protected]> wrote:
> > On Sun, 2014-01-05 at 02:36 +0530, Chaitanya T K wrote:
> >> This was blocking sending SMPS action frames
> >> through debugfs.
> >
> > I don't see any issue here, explain.
> >
> > johannes
> >
> buflen includes the new line character as well, hence the comparison
> strncmp fails for all combiantions.
>
> echo "static" > ieee80211/phyX/netdev\:wlanX/smps
> Then
>
> buf=static\n
> buflen=7
>
> But the comparison is with "static" which doesn't include "\n"
> hence the comparison fails.
Err, ok, so you can just do "echo -n static", right?
And then your patch breaks the way it currently works, which is about
the worst you can do.
johannes
On Mon, Jan 6, 2014 at 8:45 PM, Johannes Berg <[email protected]> wrote:
> On Mon, 2014-01-06 at 20:35 +0530, Krishna Chaitanya wrote:
>> On Mon, Jan 6, 2014 at 8:18 PM, Johannes Berg <[email protected]> wrote:
>> > On Sun, 2014-01-05 at 02:36 +0530, Chaitanya T K wrote:
>> >> This was blocking sending SMPS action frames
>> >> through debugfs.
>> >
>> > I don't see any issue here, explain.
>> >
>> > johannes
>> >
>> buflen includes the new line character as well, hence the comparison
>> strncmp fails for all combiantions.
>>
>> echo "static" > ieee80211/phyX/netdev\:wlanX/smps
>> Then
>>
>> buf=static\n
>> buflen=7
>>
>> But the comparison is with "static" which doesn't include "\n"
>> hence the comparison fails.
>
> Err, ok, so you can just do "echo -n static", right?
>
> And then your patch breaks the way it currently works, which is about
> the worst you can do.
>
Ok, if one works other fails.
So instead why cant we use strlen(local_string)
instead of using buflen(remote). That way we can make sure we only
compare the characters we need and leave out the extra ones.
On Sun, 2014-01-05 at 02:36 +0530, Chaitanya T K wrote:
> This was blocking sending SMPS action frames
> through debugfs.
I don't see any issue here, explain.
johannes
On Mon, Jan 6, 2014 at 9:25 PM, Johannes Berg <[email protected]> wrote:
> On Mon, 2014-01-06 at 21:02 +0530, Krishna Chaitanya wrote:
>> On Mon, Jan 6, 2014 at 8:45 PM, Johannes Berg <[email protected]> wrote:
>> > On Mon, 2014-01-06 at 20:35 +0530, Krishna Chaitanya wrote:
>> >> On Mon, Jan 6, 2014 at 8:18 PM, Johannes Berg <[email protected]> wrote:
>> >> > On Sun, 2014-01-05 at 02:36 +0530, Chaitanya T K wrote:
>> >> >> This was blocking sending SMPS action frames
>> >> >> through debugfs.
>> >> >
>> >> > I don't see any issue here, explain.
>> >> >
>> >> > johannes
>> >> >
>> >> buflen includes the new line character as well, hence the comparison
>> >> strncmp fails for all combiantions.
>> >>
>> >> echo "static" > ieee80211/phyX/netdev\:wlanX/smps
>> >> Then
>> >>
>> >> buf=static\n
>> >> buflen=7
>> >>
>> >> But the comparison is with "static" which doesn't include "\n"
>> >> hence the comparison fails.
>> >
>> > Err, ok, so you can just do "echo -n static", right?
>> >
>> > And then your patch breaks the way it currently works, which is about
>> > the worst you can do.
>> >
>> Ok, if one works other fails.
>>
>> So instead why cant we use strlen(local_string)
>> instead of using buflen(remote). That way we can make sure we only
>> compare the characters we need and leave out the extra ones.
>
> It wouldn't fix the problem and would introduce a buffer overflow bug.
>
We can add some checks to make sure it doesn't overflow, but its not
worth it.
My intention is as most of the users are familiar with "echo" and
not "echo -n", its better to have a solution which works with "echo".
Also if we use buflen-1 and give "echo -n" it still works but problem is
it compares 1 less character.
Anyways either one is fine with me.
Thanks.
On Mon, 2014-01-06 at 21:02 +0530, Krishna Chaitanya wrote:
> On Mon, Jan 6, 2014 at 8:45 PM, Johannes Berg <[email protected]> wrote:
> > On Mon, 2014-01-06 at 20:35 +0530, Krishna Chaitanya wrote:
> >> On Mon, Jan 6, 2014 at 8:18 PM, Johannes Berg <[email protected]> wrote:
> >> > On Sun, 2014-01-05 at 02:36 +0530, Chaitanya T K wrote:
> >> >> This was blocking sending SMPS action frames
> >> >> through debugfs.
> >> >
> >> > I don't see any issue here, explain.
> >> >
> >> > johannes
> >> >
> >> buflen includes the new line character as well, hence the comparison
> >> strncmp fails for all combiantions.
> >>
> >> echo "static" > ieee80211/phyX/netdev\:wlanX/smps
> >> Then
> >>
> >> buf=static\n
> >> buflen=7
> >>
> >> But the comparison is with "static" which doesn't include "\n"
> >> hence the comparison fails.
> >
> > Err, ok, so you can just do "echo -n static", right?
> >
> > And then your patch breaks the way it currently works, which is about
> > the worst you can do.
> >
> Ok, if one works other fails.
>
> So instead why cant we use strlen(local_string)
> instead of using buflen(remote). That way we can make sure we only
> compare the characters we need and leave out the extra ones.
It wouldn't fix the problem and would introduce a buffer overflow bug.
johannes