| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <ZNCnInbK2O0HZGkH@nanopsycho>
Date: Mon, 7 Aug 2023 10:11:14 +0200
From: Jiri Pirko <jiri@...nulli.us>
To: Vadim Fedorenko <vadim.fedorenko@...ux.dev>
Cc: Jakub Kicinski <kuba@...nel.org>,
Arkadiusz Kubalewski <arkadiusz.kubalewski@...el.com>,
Jonathan Lemon <jonathan.lemon@...il.com>,
Paolo Abeni <pabeni@...hat.com>,
Milena Olech <milena.olech@...el.com>,
Michal Michalik <michal.michalik@...el.com>,
linux-arm-kernel@...ts.infradead.org, poros@...hat.com,
mschmidt@...hat.com, netdev@...r.kernel.org,
linux-clk@...r.kernel.org, Bart Van Assche <bvanassche@....org>,
intel-wired-lan@...ts.osuosl.org
Subject: Re: [PATCH net-next v2 4/9] dpll: netlink: Add DPLL framework base
functions
Fri, Aug 04, 2023 at 09:04:49PM CEST, vadim.fedorenko@...ux.dev wrote:
>DPLL framework is used to represent and configure DPLL devices
>in systems. Each device that has DPLL and can configure inputs
>and outputs can use this framework.
>
>Implement dpll netlink framework functions for enablement of dpll
>subsystem netlink family.
>
>Co-developed-by: Milena Olech <milena.olech@...el.com>
>Signed-off-by: Milena Olech <milena.olech@...el.com>
>Co-developed-by: Michal Michalik <michal.michalik@...el.com>
>Signed-off-by: Michal Michalik <michal.michalik@...el.com>
>Signed-off-by: Vadim Fedorenko <vadim.fedorenko@...ux.dev>
>Co-developed-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@...el.com>
>Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@...el.com>
Besides couple of small nits below, looks fine to me.
Signed-off-by: Jiri Pirko <jiri@...dia.com>
[...]
>+static int
>+dpll_pin_freq_set(struct dpll_pin *pin, struct nlattr *a,
>+ struct netlink_ext_ack *extack)
>+{
>+ u64 freq = nla_get_u64(a);
>+ struct dpll_pin_ref *ref;
>+ unsigned long i;
>+ int ret;
>+
>+ if (!dpll_pin_is_freq_supported(pin, freq)) {
>+ NL_SET_ERR_MSG_FMT(extack, "not supported freq:%llu on pin:%u",
Please remove "pin%u". User knows on which object he is operating.
>+ freq, pin->id);
>+ return -EINVAL;
>+ }
>+
>+ xa_for_each(&pin->dpll_refs, i, ref) {
>+ const struct dpll_pin_ops *ops = dpll_pin_ops(ref);
>+ struct dpll_device *dpll = ref->dpll;
>+
>+ if (!ops->frequency_set)
>+ return -EOPNOTSUPP;
>+ ret = ops->frequency_set(pin, dpll_pin_on_dpll_priv(dpll, pin),
>+ dpll, dpll_priv(dpll), freq, extack);
>+ if (ret)
>+ return ret;
>+ __dpll_pin_change_ntf(pin);
>+ }
>+
>+ return 0;
>+}
>+
>+static int
>+dpll_pin_on_pin_state_set(struct dpll_pin *pin, u32 parent_idx,
>+ enum dpll_pin_state state,
>+ struct netlink_ext_ack *extack)
>+{
>+ struct dpll_pin_ref *parent_ref;
>+ const struct dpll_pin_ops *ops;
>+ struct dpll_pin_ref *dpll_ref;
>+ void *pin_priv, *parent_priv;
>+ struct dpll_pin *parent;
>+ unsigned long i;
>+ int ret;
>+
>+ if (!(DPLL_PIN_CAPS_STATE_CAN_CHANGE & pin->prop->capabilities)) {
>+ NL_SET_ERR_MSG_FMT(extack, "pin:%u not allowed to change state",
Please remove "pin%u". User knows on which object he is operating.
>+ pin->id);
>+ return -EOPNOTSUPP;
>+ }
>+ parent = xa_load(&dpll_pin_xa, parent_idx);
>+ if (!parent)
>+ return -EINVAL;
>+ parent_ref = xa_load(&pin->parent_refs, parent->pin_idx);
>+ if (!parent_ref)
>+ return -EINVAL;
>+ xa_for_each(&parent->dpll_refs, i, dpll_ref) {
>+ ops = dpll_pin_ops(parent_ref);
>+ if (!ops->state_on_pin_set)
>+ return -EOPNOTSUPP;
>+ pin_priv = dpll_pin_on_pin_priv(parent, pin);
>+ parent_priv = dpll_pin_on_dpll_priv(dpll_ref->dpll, parent);
>+ ret = ops->state_on_pin_set(pin, pin_priv, parent, parent_priv,
>+ state, extack);
>+ if (ret)
>+ return ret;
>+ }
>+ __dpll_pin_change_ntf(pin);
>+
>+ return 0;
>+}
>+
>+static int
>+dpll_pin_state_set(struct dpll_device *dpll, struct dpll_pin *pin,
>+ enum dpll_pin_state state,
>+ struct netlink_ext_ack *extack)
>+{
>+ const struct dpll_pin_ops *ops;
>+ struct dpll_pin_ref *ref;
>+ int ret;
>+
>+ if (!(DPLL_PIN_CAPS_STATE_CAN_CHANGE & pin->prop->capabilities)) {
>+ NL_SET_ERR_MSG_FMT(extack, "pin:%u not allowed to change state",
Please remove "pin%u". User knows on which object he is operating.
>+ pin->id);
>+ return -EOPNOTSUPP;
>+ }
>+ ref = xa_load(&pin->dpll_refs, dpll->device_idx);
>+ ASSERT_NOT_NULL(ref);
>+ ops = dpll_pin_ops(ref);
>+ if (!ops->state_on_dpll_set)
>+ return -EOPNOTSUPP;
>+ ret = ops->state_on_dpll_set(pin, dpll_pin_on_dpll_priv(dpll, pin),
>+ dpll, dpll_priv(dpll), state, extack);
>+ if (ret)
>+ return ret;
>+ __dpll_pin_change_ntf(pin);
>+
>+ return 0;
>+}
>+
>+static int
>+dpll_pin_prio_set(struct dpll_device *dpll, struct dpll_pin *pin,
>+ u32 prio, struct netlink_ext_ack *extack)
>+{
>+ const struct dpll_pin_ops *ops;
>+ struct dpll_pin_ref *ref;
>+ int ret;
>+
>+ if (!(DPLL_PIN_CAPS_PRIORITY_CAN_CHANGE & pin->prop->capabilities)) {
>+ NL_SET_ERR_MSG_FMT(extack, "pin:%u not allowed to change prio",
Please remove "pin%u". User knows on which object he is operating.
>+ pin->id);
>+ return -EOPNOTSUPP;
>+ }
>+ ref = xa_load(&pin->dpll_refs, dpll->device_idx);
>+ ASSERT_NOT_NULL(ref);
>+ ops = dpll_pin_ops(ref);
>+ if (!ops->prio_set)
>+ return -EOPNOTSUPP;
>+ ret = ops->prio_set(pin, dpll_pin_on_dpll_priv(dpll, pin), dpll,
>+ dpll_priv(dpll), prio, extack);
>+ if (ret)
>+ return ret;
>+ __dpll_pin_change_ntf(pin);
>+
>+ return 0;
>+}
>+
>+static int
>+dpll_pin_direction_set(struct dpll_pin *pin, struct dpll_device *dpll,
>+ enum dpll_pin_direction direction,
>+ struct netlink_ext_ack *extack)
>+{
>+ const struct dpll_pin_ops *ops;
>+ struct dpll_pin_ref *ref;
>+ int ret;
>+
>+ if (!(DPLL_PIN_CAPS_DIRECTION_CAN_CHANGE & pin->prop->capabilities)) {
>+ NL_SET_ERR_MSG_FMT(extack,
>+ "pin:%u not allowed to change direction",
Please remove "pin%u". User knows on which object he is operating.
>+ pin->id);
>+ return -EOPNOTSUPP;
>+ }
>+ ref = xa_load(&pin->dpll_refs, dpll->device_idx);
>+ ASSERT_NOT_NULL(ref);
>+ ops = dpll_pin_ops(ref);
>+ if (!ops->direction_set)
>+ return -EOPNOTSUPP;
>+ ret = ops->direction_set(pin, dpll_pin_on_dpll_priv(dpll, pin),
>+ dpll, dpll_priv(dpll), direction, extack);
>+ if (ret)
>+ return ret;
>+ __dpll_pin_change_ntf(pin);
>+
>+ return 0;
>+}
>+
[...]
>+static int
>+dpll_pin_set_from_nlattr(struct dpll_pin *pin, struct genl_info *info)
>+{
>+ int rem, ret = -EINVAL;
>+ struct nlattr *a;
>+
>+ nla_for_each_attr(a, genlmsg_data(info->genlhdr),
>+ genlmsg_len(info->genlhdr), rem) {
>+ switch (nla_type(a)) {
>+ case DPLL_A_PIN_FREQUENCY:
>+ ret = dpll_pin_freq_set(pin, a, info->extack);
>+ if (ret)
>+ return ret;
>+ break;
>+ case DPLL_A_PIN_PARENT_DEVICE:
>+ ret = dpll_pin_parent_device_set(pin, a, info->extack);
>+ if (ret)
>+ return ret;
>+ break;
>+ case DPLL_A_PIN_PARENT_PIN:
>+ ret = dpll_pin_parent_pin_set(pin, a, info->extack);
>+ if (ret)
>+ return ret;
>+ break;
>+ case DPLL_A_PIN_ID:
>+ case DPLL_A_ID:
>+ break;
>+ default:
>+ NL_SET_ERR_MSG_FMT(info->extack,
>+ "unsupported attribute (%d)",
>+ nla_type(a));
How exactly this could happen? I mean, we have strict parsing on, the
policy checking during parse ensures no unknown attr is seen here.
Please remove this "default" section.
>+ return -EINVAL;
>+ }
>+ }
>+
>+ return 0;
>+}
>+
[...]
Powered by blists - more mailing lists