From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from lists.s-osg.org ([54.187.51.154]:58504 "EHLO lists.s-osg.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751658AbbHSO2h (ORCPT ); Wed, 19 Aug 2015 10:28:37 -0400 Subject: Re: [PATCH wpan-tools] mac: range checking for command accepting only 0 or 1 References: <1439993604-8335-1-git-send-email-stefan@osg.samsung.com> <20150819142147.GA31917@omega> From: Stefan Schmidt Message-ID: <55D49291.6090702@osg.samsung.com> Date: Wed, 19 Aug 2015 16:28:33 +0200 MIME-Version: 1.0 In-Reply-To: <20150819142147.GA31917@omega> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-wpan-owner@vger.kernel.org List-ID: To: Alexander Aring Cc: linux-wpan@vger.kernel.org Hello. On 19/08/15 16:22, Alexander Aring wrote: > Hi Stefan, > > On Wed, Aug 19, 2015 at 04:13:24PM +0200, Stefan Schmidt wrote: >> lbt and ackreq_default only accept 0or 1 as arguments which we did not >> acount for so far. Testing invalid arguments and checking teh return >> code uncovered this one. >> >> Signed-off-by: Stefan Schmidt >> --- >> src/mac.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/src/mac.c b/src/mac.c >> index 76db58f..26c6fc5 100644 >> --- a/src/mac.c >> +++ b/src/mac.c >> @@ -175,6 +175,8 @@ static int handle_lbt_mode(struct nl802154_state *state, >> mode = strtoul(argv[0], &end, 0); >> if (*end != '\0') >> return 1; >> + if (mode != 0 && mode != 1) >> + return 1; >> >> NLA_PUT_U8(msg, NL802154_ATTR_LBT_MODE, mode); >> >> @@ -202,6 +204,8 @@ static int handle_ackreq_default(struct nl802154_state *state, >> ackreq = strtoul(argv[0], &end, 0); >> if (*end != '\0') >> return 1; >> + if (ackreq != 0 && ackreq != 1) >> + return 1; >> > range checks should be handled by kernelspace. Currently we do a "!!var" > conversion there. Which is why any value > 0 gets set to 1 here. > Maybe we should change it there and accept "1" or "0". I think that would indeed be better. > We should never handle "if a range is valid or not" inside userspace, > otherwise other applications which talking to nl802154 can do a > different handling then. I disagree here. We should for sure do range checking in the kernel but we should as well check for it in iwpan and get the return code right. regards Stefan Schmidt