From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1161087AbcFIWAb (ORCPT ); Thu, 9 Jun 2016 18:00:31 -0400 Received: from youngberry.canonical.com ([91.189.89.112]:50920 "EHLO youngberry.canonical.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752894AbcFIVSZ (ORCPT ); Thu, 9 Jun 2016 17:18:25 -0400 From: Kamal Mostafa To: linux-kernel@vger.kernel.org, stable@vger.kernel.org, kernel-team@lists.ubuntu.com Cc: Oliver Hartkopp , Marc Kleine-Budde , Kamal Mostafa Subject: [PATCH 4.2.y-ckt 069/206] can: fix handling of unmodifiable configuration options Date: Thu, 9 Jun 2016 14:14:38 -0700 Message-Id: <1465507015-23052-70-git-send-email-kamal@canonical.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1465507015-23052-1-git-send-email-kamal@canonical.com> References: <1465507015-23052-1-git-send-email-kamal@canonical.com> X-Extended-Stable: 4.2 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 4.2.8-ckt12 -stable review patch. If anyone has any objections, please let me know. ---8<------------------------------------------------------------ From: Oliver Hartkopp commit bb208f144cf3f59d8f89a09a80efd04389718907 upstream. As described in 'can: m_can: tag current CAN FD controllers as non-ISO' (6cfda7fbebe) it is possible to define fixed configuration options by setting the according bit in 'ctrlmode' and clear it in 'ctrlmode_supported'. This leads to the incovenience that the fixed configuration bits can not be passed by netlink even when they have the correct values (e.g. non-ISO, FD). This patch fixes that issue and not only allows fixed set bit values to be set again but now requires(!) to provide these fixed values at configuration time. A valid CAN FD configuration consists of a nominal/arbitration bittiming, a data bittiming and a control mode with CAN_CTRLMODE_FD set - which is now enforced by a new can_validate() function. This fix additionally removed the inconsistency that was prohibiting the support of 'CANFD-only' controller drivers, like the RCar CAN FD. For this reason a new helper can_set_static_ctrlmode() has been introduced to provide a proper interface to handle static enabled CAN controller options. Reported-by: Ramesh Shanmugasundaram Signed-off-by: Oliver Hartkopp Reviewed-by: Ramesh Shanmugasundaram Signed-off-by: Marc Kleine-Budde Signed-off-by: Kamal Mostafa --- drivers/net/can/dev.c | 56 +++++++++++++++++++++++++++++++++++++++---- drivers/net/can/m_can/m_can.c | 2 +- include/linux/can/dev.h | 22 +++++++++++++++-- 3 files changed, 73 insertions(+), 7 deletions(-) diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c index 141c2a4..910c12e 100644 --- a/drivers/net/can/dev.c +++ b/drivers/net/can/dev.c @@ -696,11 +696,17 @@ int can_change_mtu(struct net_device *dev, int new_mtu) /* allow change of MTU according to the CANFD ability of the device */ switch (new_mtu) { case CAN_MTU: + /* 'CANFD-only' controllers can not switch to CAN_MTU */ + if (priv->ctrlmode_static & CAN_CTRLMODE_FD) + return -EINVAL; + priv->ctrlmode &= ~CAN_CTRLMODE_FD; break; case CANFD_MTU: - if (!(priv->ctrlmode_supported & CAN_CTRLMODE_FD)) + /* check for potential CANFD ability */ + if (!(priv->ctrlmode_supported & CAN_CTRLMODE_FD) && + !(priv->ctrlmode_static & CAN_CTRLMODE_FD)) return -EINVAL; priv->ctrlmode |= CAN_CTRLMODE_FD; @@ -782,6 +788,35 @@ static const struct nla_policy can_policy[IFLA_CAN_MAX + 1] = { = { .len = sizeof(struct can_bittiming_const) }, }; +static int can_validate(struct nlattr *tb[], struct nlattr *data[]) +{ + bool is_can_fd = false; + + /* Make sure that valid CAN FD configurations always consist of + * - nominal/arbitration bittiming + * - data bittiming + * - control mode with CAN_CTRLMODE_FD set + */ + + if (data[IFLA_CAN_CTRLMODE]) { + struct can_ctrlmode *cm = nla_data(data[IFLA_CAN_CTRLMODE]); + + is_can_fd = cm->flags & cm->mask & CAN_CTRLMODE_FD; + } + + if (is_can_fd) { + if (!data[IFLA_CAN_BITTIMING] || !data[IFLA_CAN_DATA_BITTIMING]) + return -EOPNOTSUPP; + } + + if (data[IFLA_CAN_DATA_BITTIMING]) { + if (!is_can_fd || !data[IFLA_CAN_BITTIMING]) + return -EOPNOTSUPP; + } + + return 0; +} + static int can_changelink(struct net_device *dev, struct nlattr *tb[], struct nlattr *data[]) { @@ -813,19 +848,31 @@ static int can_changelink(struct net_device *dev, if (data[IFLA_CAN_CTRLMODE]) { struct can_ctrlmode *cm; + u32 ctrlstatic; + u32 maskedflags; /* Do not allow changing controller mode while running */ if (dev->flags & IFF_UP) return -EBUSY; cm = nla_data(data[IFLA_CAN_CTRLMODE]); + ctrlstatic = priv->ctrlmode_static; + maskedflags = cm->flags & cm->mask; + + /* check whether provided bits are allowed to be passed */ + if (cm->mask & ~(priv->ctrlmode_supported | ctrlstatic)) + return -EOPNOTSUPP; + + /* do not check for static fd-non-iso if 'fd' is disabled */ + if (!(maskedflags & CAN_CTRLMODE_FD)) + ctrlstatic &= ~CAN_CTRLMODE_FD_NON_ISO; - /* check whether changed bits are allowed to be modified */ - if (cm->mask & ~priv->ctrlmode_supported) + /* make sure static options are provided by configuration */ + if ((maskedflags & ctrlstatic) != ctrlstatic) return -EOPNOTSUPP; /* clear bits to be modified and copy the flag values */ priv->ctrlmode &= ~cm->mask; - priv->ctrlmode |= (cm->flags & cm->mask); + priv->ctrlmode |= maskedflags; /* CAN_CTRLMODE_FD can only be set when driver supports FD */ if (priv->ctrlmode & CAN_CTRLMODE_FD) @@ -966,6 +1013,7 @@ static struct rtnl_link_ops can_link_ops __read_mostly = { .maxtype = IFLA_CAN_MAX, .policy = can_policy, .setup = can_setup, + .validate = can_validate, .newlink = can_newlink, .changelink = can_changelink, .get_size = can_get_size, diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c index ef65517..37f15eb 100644 --- a/drivers/net/can/m_can/m_can.c +++ b/drivers/net/can/m_can/m_can.c @@ -958,7 +958,7 @@ static struct net_device *alloc_m_can_dev(void) priv->can.do_get_berr_counter = m_can_get_berr_counter; /* CAN_CTRLMODE_FD_NON_ISO is fixed with M_CAN IP v3.0.1 */ - priv->can.ctrlmode = CAN_CTRLMODE_FD_NON_ISO; + can_set_static_ctrlmode(dev, CAN_CTRLMODE_FD_NON_ISO); /* CAN_CTRLMODE_FD_NON_ISO can not be changed with M_CAN IP v3.0.1 */ priv->can.ctrlmode_supported = CAN_CTRLMODE_LOOPBACK | diff --git a/include/linux/can/dev.h b/include/linux/can/dev.h index c3a9c8f..5e13b98 100644 --- a/include/linux/can/dev.h +++ b/include/linux/can/dev.h @@ -39,8 +39,11 @@ struct can_priv { struct can_clock clock; enum can_state state; - u32 ctrlmode; - u32 ctrlmode_supported; + + /* CAN controller features - see include/uapi/linux/can/netlink.h */ + u32 ctrlmode; /* current options setting */ + u32 ctrlmode_supported; /* options that can be modified by netlink */ + u32 ctrlmode_static; /* static enabled options for driver/hardware */ int restart_ms; struct timer_list restart_timer; @@ -107,6 +110,21 @@ static inline bool can_is_canfd_skb(const struct sk_buff *skb) return skb->len == CANFD_MTU; } +/* helper to define static CAN controller features at device creation time */ +static inline void can_set_static_ctrlmode(struct net_device *dev, + u32 static_mode) +{ + struct can_priv *priv = netdev_priv(dev); + + /* alloc_candev() succeeded => netdev_priv() is valid at this point */ + priv->ctrlmode = static_mode; + priv->ctrlmode_static = static_mode; + + /* override MTU which was set by default in can_setup()? */ + if (static_mode & CAN_CTRLMODE_FD) + dev->mtu = CANFD_MTU; +} + /* get data length from can_dlc with sanitized can_dlc */ u8 can_dlc2len(u8 can_dlc); -- 2.7.4