From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Message-ID: <542BA324.7010603@lab.ntt.co.jp> Date: Wed, 01 Oct 2014 15:45:56 +0900 From: Toshiaki Makita MIME-Version: 1.0 References: <1412105462-340-1-git-send-email-vyasevic@redhat.com> <1412105462-340-2-git-send-email-vyasevic@redhat.com> In-Reply-To: <1412105462-340-2-git-send-email-vyasevic@redhat.com> Content-Type: text/plain; charset=iso-2022-jp Content-Transfer-Encoding: 7bit Subject: Re: [Bridge] [PATCH v2 net 1/3] bridge: Add a default_pvid sysfs attribute List-Id: Linux Ethernet Bridging List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Vladislav Yasevich , netdev@vger.kernel.org Cc: stephen@networkplumber.org, bridge@lists.linux-foundation.org, Vladislav Yasevich On 2014/10/01 4:31, Vladislav Yasevich wrote: > This patch allows the user to set and retrieve default_pvid > value. A new value can only be stored when vlan filtering > is disabled. > > Signed-off-by: Vladislav Yasevich > --- ... > +int br_vlan_set_default_pvid(struct net_bridge *br, unsigned long val) > +{ > + u16 pvid = val; > + int err = 0; > + > + if (!pvid || pvid >= VLAN_VID_MASK) > + return -EINVAL; This seems to accept a large value as a valid value. For example, ((1 << 16) + 10) will be handled as 10. How about using "val" for this check? > + > + if (!rtnl_trylock()) > + return restart_syscall(); > + > + if (pvid == br->default_pvid) > + goto unlock; > + > + /* Only allow default pvid change when filtering is disabled */ > + if (br->vlan_enabled) { > + err = -EPERM; Additional log message might help users know why it was rejected. Thanks, Toshiaki Makita From mboxrd@z Thu Jan 1 00:00:00 1970 From: Toshiaki Makita Subject: Re: [PATCH v2 net 1/3] bridge: Add a default_pvid sysfs attribute Date: Wed, 01 Oct 2014 15:45:56 +0900 Message-ID: <542BA324.7010603@lab.ntt.co.jp> References: <1412105462-340-1-git-send-email-vyasevic@redhat.com> <1412105462-340-2-git-send-email-vyasevic@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=iso-2022-jp Content-Transfer-Encoding: 7bit Cc: stephen@networkplumber.org, bridge@lists.linux-foundation.org, Vladislav Yasevich To: Vladislav Yasevich , netdev@vger.kernel.org Return-path: In-Reply-To: <1412105462-340-2-git-send-email-vyasevic@redhat.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: bridge-bounces@lists.linux-foundation.org Errors-To: bridge-bounces@lists.linux-foundation.org List-Id: netdev.vger.kernel.org On 2014/10/01 4:31, Vladislav Yasevich wrote: > This patch allows the user to set and retrieve default_pvid > value. A new value can only be stored when vlan filtering > is disabled. > > Signed-off-by: Vladislav Yasevich > --- ... > +int br_vlan_set_default_pvid(struct net_bridge *br, unsigned long val) > +{ > + u16 pvid = val; > + int err = 0; > + > + if (!pvid || pvid >= VLAN_VID_MASK) > + return -EINVAL; This seems to accept a large value as a valid value. For example, ((1 << 16) + 10) will be handled as 10. How about using "val" for this check? > + > + if (!rtnl_trylock()) > + return restart_syscall(); > + > + if (pvid == br->default_pvid) > + goto unlock; > + > + /* Only allow default pvid change when filtering is disabled */ > + if (br->vlan_enabled) { > + err = -EPERM; Additional log message might help users know why it was rejected. Thanks, Toshiaki Makita