| 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: <20180306085530.7b51aa29@vento.lan>
Date: Tue, 6 Mar 2018 08:55:30 -0300
From: Mauro Carvalho Chehab <mchehab@...nel.org>
To: David Howells <dhowells@...hat.com>
Cc: linux-kernel@...r.kernel.org, linux-media@...r.kernel.org
Subject: Re: [PATCH] dvb: Save port number and provide sysfs attributes to
pass values to udev
Hi David,
Em Wed, 10 Jan 2018 14:50:35 +0000
David Howells <dhowells@...hat.com> escreveu:
> Some devices, such as the DVBSky S952 and T982 cards, are dual port cards
> that provide two cx23885 devices on the same PCI device, which means the
> attributes available for writing udev rules are exactly the same, apart
> from the adapter number. Unfortunately, the adapter numbers are dependent
> on the order in which things are initialised, so this can change over
> different releases of the kernel.
>
> The struct cx23885_tsport has a port number available, which is printed
> during boot:
>
> [ 10.951517] DVBSky T982 port 1 MAC address: 00:17:42:54:09:87
> ...
> [ 10.984875] DVBSky T982 port 2 MAC address: 00:17:42:54:09:88
>
> To make it possible to distinguish these in udev, do the following steps:
>
> (1) Save the port number into struct dvb_adapter.
>
> (2) Provide sysfs attributes to export port number and also MAC address,
> adapter number and type. There are other fields that could perhaps be
> exported also.
>
> The new sysfs attributes can be seen from userspace as:
>
> [root@...eb ~]# ls /sys/class/dvb/dvb0.frontend0/
> dev device dvb_adapter dvb_mac dvb_port dvb_type
> power subsystem uevent
> [root@...eb ~]# cat /sys/class/dvb/dvb0.frontend0/dvb_*
> 0
> 00:17:42:54:09:87
> 0
> frontend
>
> They can be used in udev rules:
>
> SUBSYSTEM=="dvb", ATTRS{vendor}=="0x14f1", ATTRS{device}=="0x8852", ATTRS{subsystem_device}=="0x0982", ATTR{dvb_mac}=="00:17:42:54:09:87", PROGRAM="/bin/sh -c 'K=%k; K=$${K#dvb}; printf dvb/adapter9820/%%s $${K#*.}'", SYMLINK+="%c"
> SUBSYSTEM=="dvb", ATTRS{vendor}=="0x14f1", ATTRS{device}=="0x8852", ATTRS{subsystem_device}=="0x0982", ATTR{dvb_mac}=="00:17:42:54:09:88", PROGRAM="/bin/sh -c 'K=%k; K=$${K#dvb}; printf dvb/adapter9821/%%s $${K#*.}'", SYMLINK+="%c"
>
> where the match is made with ATTR{dvb_mac} or similar. The rules above
> make symlinks from /dev/dvb/adapter982/* to /dev/dvb/adapterXX/*.
>
> Note that binding the dvb-net device to a network interface and changing it
> there does not reflect back into the the dvb_adapter struct and doesn't
> change the MAC address here. This means that a system with two identical
> cards in it may need to distinguish them by some other means than MAC
> address.
Sorry for not looking on it earlier... Has been very busy those days,
and the dvb sub-maintainer has not been responsive lately, due to some
personal issues.
>
> Signed-off-by: David Howells <dhowells@...hat.com>
> ---
>
> drivers/media/dvb-core/dvbdev.c | 46 +++++++++++++++++++++++++++++++
> drivers/media/dvb-core/dvbdev.h | 2 +
> drivers/media/pci/cx23885/cx23885-dvb.c | 2 +
> 3 files changed, 50 insertions(+)
>
> diff --git a/drivers/media/dvb-core/dvbdev.c b/drivers/media/dvb-core/dvbdev.c
> index 060c60ddfcc3..b3aa5ae3d57f 100644
> --- a/drivers/media/dvb-core/dvbdev.c
> +++ b/drivers/media/dvb-core/dvbdev.c
> @@ -941,6 +941,51 @@ int dvb_usercopy(struct file *file,
> return err;
> }
>
> +static ssize_t dvb_adapter_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct dvb_device *dvbdev = dev_get_drvdata(dev);
> +
> + return sprintf(buf, "%d\n", dvbdev->adapter->num);
> +}
> +static DEVICE_ATTR_RO(dvb_adapter);
> +
> +static ssize_t dvb_mac_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct dvb_device *dvbdev = dev_get_drvdata(dev);
> +
> + return sprintf(buf, "%pM\n", dvbdev->adapter->proposed_mac);
> +}
> +static DEVICE_ATTR_RO(dvb_mac);
> +
> +static ssize_t dvb_port_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct dvb_device *dvbdev = dev_get_drvdata(dev);
> +
> + return sprintf(buf, "%d\n", dvbdev->adapter->port_num);
> +}
> +static DEVICE_ATTR_RO(dvb_port);
> +
> +static ssize_t dvb_type_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct dvb_device *dvbdev = dev_get_drvdata(dev);
> +
> + return sprintf(buf, "%s\n", dnames[dvbdev->type]);
> +}
> +static DEVICE_ATTR_RO(dvb_type);
> +
> +static struct attribute *dvb_class_attrs[] = {
> + &dev_attr_dvb_adapter.attr,
> + &dev_attr_dvb_mac.attr,
> + &dev_attr_dvb_port.attr,
> + &dev_attr_dvb_type.attr,
> + NULL
> +};
> +ATTRIBUTE_GROUPS(dvb_class);
> +
> static int dvb_uevent(struct device *dev, struct kobj_uevent_env *env)
> {
> struct dvb_device *dvbdev = dev_get_drvdata(dev);
> @@ -981,6 +1026,7 @@ static int __init init_dvbdev(void)
> retval = PTR_ERR(dvb_class);
> goto error;
> }
> + dvb_class->dev_groups = dvb_class_groups,
> dvb_class->dev_uevent = dvb_uevent;
> dvb_class->devnode = dvb_devnode;
> return 0;
The patch itself looks good, but I'm not seeing any documentation.
You should likely add something to Documentation/ABI and to the
DVB uAPI (Documentation/media/uapi/dvb).
> diff --git a/drivers/media/dvb-core/dvbdev.h b/drivers/media/dvb-core/dvbdev.h
> index bbc1c20c0529..1d5a170e279a 100644
> --- a/drivers/media/dvb-core/dvbdev.h
> +++ b/drivers/media/dvb-core/dvbdev.h
> @@ -83,6 +83,7 @@ struct dvb_frontend;
> * @device_list: List with the DVB devices
> * @name: Name of the adapter
> * @proposed_mac: proposed MAC address for the adapter
> + * @port_num: Port number for multi-adapter devices
> * @priv: private data
> * @device: pointer to struct device
> * @module: pointer to struct module
> @@ -103,6 +104,7 @@ struct dvb_adapter {
> struct list_head device_list;
> const char *name;
> u8 proposed_mac [6];
> + u8 port_num;
> void* priv;
>
> struct device *device;
> diff --git a/drivers/media/pci/cx23885/cx23885-dvb.c b/drivers/media/pci/cx23885/cx23885-dvb.c
> index e795ddeb7fe2..19c72c66d7e0 100644
> --- a/drivers/media/pci/cx23885/cx23885-dvb.c
> +++ b/drivers/media/pci/cx23885/cx23885-dvb.c
> @@ -1217,6 +1217,8 @@ static int dvb_register(struct cx23885_tsport *port)
> /* Sets the gate control callback to be used by i2c command calls */
> port->gate_ctrl = cx23885_dvb_gate_ctrl;
>
> + port->frontends.adapter.port_num = port->nr;
> +
Doing it for each multi-adapter device is something that bothers
me. The better would be if we could move this to the DVB Kernel,
in order to not need to check/fix every driver. If, otherwise,
this is not possible, then we need a patch fixing port_num for
all drivers that support multiple adapters.
Also, the risk of forgetting it seems high. So, perhaps we should
add a new parameter to some function (like at dvb_register_device
or at dvb_register_frontend), in order to make the port number
a mandatory attribute.
> /* init frontend */
> switch (dev->board) {
> case CX23885_BOARD_HAUPPAUGE_HVR1250:
>
Thanks,
Mauro
Powered by blists - more mailing lists