| 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: <5318A2E8.9020500@samsung.com>
Date: Thu, 06 Mar 2014 09:31:36 -0700
From: Shuah Khan <shuah.kh@...sung.com>
To: Valentina Manea <valentina.manea.m@...il.com>,
gregkh@...uxfoundation.org
Cc: tobias.polzer@....de, dominik.paulus@....de,
ly80toro@....cs.fau.de, ihadzic@...earch.bell-labs.com,
linux-kernel@...r.kernel.org, linux-usb@...r.kernel.org,
devel@...verdev.osuosl.org, firefly@...ts.rosedu.org,
Shuah Khan <shuah.kh@...sung.com>
Subject: Re: [PATCH 03/12] staging: usbip: userspace: migrate usbip_unbind to
libudev
On 03/04/2014 12:10 PM, Valentina Manea wrote:
> This patch modifies usbip_unbind to use libudev.
>
> Signed-off-by: Valentina Manea <valentina.manea.m@...il.com>
> ---
> drivers/staging/usbip/userspace/src/usbip_unbind.c | 92 +++++++---------------
> 1 file changed, 29 insertions(+), 63 deletions(-)
>
> diff --git a/drivers/staging/usbip/userspace/src/usbip_unbind.c b/drivers/staging/usbip/userspace/src/usbip_unbind.c
> index cace878..4776068 100644
> --- a/drivers/staging/usbip/userspace/src/usbip_unbind.c
> +++ b/drivers/staging/usbip/userspace/src/usbip_unbind.c
> @@ -16,7 +16,7 @@
> * along with this program. If not, see <http://www.gnu.org/licenses/>.
> */
>
> -#include <sysfs/libsysfs.h>
> +#include <libudev.h>
>
> #include <errno.h>
> #include <stdio.h>
> @@ -27,6 +27,7 @@
> #include "usbip_common.h"
> #include "utils.h"
> #include "usbip.h"
> +#include "sysfs_utils.h"
>
> static const char usbip_unbind_usage_string[] =
> "usbip unbind <args>\n"
> @@ -41,92 +42,57 @@ void usbip_unbind_usage(void)
> static int unbind_device(char *busid)
> {
> char bus_type[] = "usb";
> - struct sysfs_driver *usbip_host_drv;
> - struct sysfs_device *dev;
> - struct dlist *devlist;
> - int verified = 0;
> int rc, ret = -1;
>
> char attr_name[] = "unbind";
> - char sysfs_mntpath[SYSFS_PATH_MAX];
> char unbind_attr_path[SYSFS_PATH_MAX];
> - struct sysfs_attribute *unbind_attr;
> -
> - /* verify the busid device is using usbip-host */
> - usbip_host_drv = sysfs_open_driver(bus_type, USBIP_HOST_DRV_NAME);
> - if (!usbip_host_drv) {
> - err("could not open %s driver: %s", USBIP_HOST_DRV_NAME,
> - strerror(errno));
> - return -1;
> - }
>
> - devlist = sysfs_get_driver_devices(usbip_host_drv);
> - if (!devlist) {
> - err("%s is not in use by any devices", USBIP_HOST_DRV_NAME);
> - goto err_close_usbip_host_drv;
> - }
> + struct udev *udev;
> + struct udev_device *dev;
> + const char *driver;
>
> - dlist_for_each_data(devlist, dev, struct sysfs_device) {
> - if (!strncmp(busid, dev->name, strlen(busid)) &&
> - !strncmp(dev->driver_name, USBIP_HOST_DRV_NAME,
> - strlen(USBIP_HOST_DRV_NAME))) {
> - verified = 1;
> - break;
> - }
> - }
> + /* Create libudev context. */
> + udev = udev_new();
>
> - if (!verified) {
> - err("device on busid %s is not using %s", busid,
> - USBIP_HOST_DRV_NAME);
> - goto err_close_usbip_host_drv;
> + /* Check whether the device with this bus ID exists. */
> + dev = udev_device_new_from_subsystem_sysname(udev, "usb", busid);
> + if (!dev) {
> + err("Device with the specified bus ID does not exist.");
> + goto err_close_udev;
> }
>
> - /*
> - * NOTE: A read and write of an attribute value of the device busid
> - * refers to must be done to start probing. That way a rebind of the
> - * default driver for the device occurs.
> - *
> - * This seems very hackish and adds a lot of pointless code. I think it
> - * should be done in the kernel by the driver after del_match_busid is
> - * finished!
> - */
> -
> - rc = sysfs_get_mnt_path(sysfs_mntpath, SYSFS_PATH_MAX);
> - if (rc < 0) {
> - err("sysfs must be mounted: %s", strerror(errno));
> - return -1;
> + /* Check whether the device is using usbip-host driver. */
> + driver = udev_device_get_driver(dev);
> + if (!driver || strcmp(driver, "usbip-host")) {
> + err("Device is not bound to usbip-host driver.");
> + goto err_close_udev;
> }
>
> + /* Unbind device from driver. */
> snprintf(unbind_attr_path, sizeof(unbind_attr_path), "%s/%s/%s/%s/%s/%s",
> - sysfs_mntpath, SYSFS_BUS_NAME, bus_type, SYSFS_DRIVERS_NAME,
> + SYSFS_MNT_PATH, SYSFS_BUS_NAME, bus_type, SYSFS_DRIVERS_NAME,
> USBIP_HOST_DRV_NAME, attr_name);
> + dbg("unbind attribute path: %s", unbind_attr_path);
Could you please remove this debug message.
>
> - /* read a device attribute */
> - unbind_attr = sysfs_open_attribute(unbind_attr_path);
> - if (!unbind_attr) {
> - err("could not open %s/%s: %s", busid, attr_name,
> - strerror(errno));
> - return -1;
> + rc = write_sysfs_attribute(unbind_attr_path, busid, strlen(busid));
> + if (rc < 0) {
> + dbg("Error unbinding device %s from driver.", busid);
Could you please make this an err()
> + goto err_close_udev;
> }
>
> - /* notify driver of unbind */
> + /* Notify driver of unbind. */
> rc = modify_match_busid(busid, 0);
> if (rc < 0) {
> err("unable to unbind device on %s", busid);
> + goto err_close_udev;
> }
>
> - rc = sysfs_write_attribute(unbind_attr, busid,
> - SYSFS_BUS_ID_SIZE);
> - if (rc < 0) {
> - dbg("bind driver at %s failed", busid);
> - }
> - sysfs_close_attribute(unbind_attr);
> -
> ret = 0;
> printf("unbind device on busid %s: complete\n", busid);
Could you please change this to an info()
>
> -err_close_usbip_host_drv:
> - sysfs_close_driver(usbip_host_drv);
> +err_close_udev:
> + udev_device_unref(dev);
> + udev_unref(udev);
>
> return ret;
> }
>
You have my Reviewed-by after making the recommended changes.
Reviewed-by: Shuah Khan <shuah.kh@...sung.com>
-- Shuah
--
Shuah Khan
Senior Linux Kernel Developer - Open Source Group
Samsung Research America(Silicon Valley)
shuah.kh@...sung.com | (970) 672-0658
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists