Re: [PATCH 1/3] usb: gadget: function: 9pfs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Le 16/01/2024 à 02:49, Michael Grzeschik a écrit :
Add the new gadget function for 9pfs transport. This function is
defining an simple 9pfs transport interface that consists of one in and
one out endpoint. The endpoints transmit and receive the 9pfs protocol
payload when mounting a 9p filesystem over usb.

Signed-off-by: Michael Grzeschik <m.grzeschik@xxxxxxxxxxxxxx>
---
  Documentation/filesystems/9p.rst     |  12 +
  drivers/usb/gadget/Kconfig           |  11 +
  drivers/usb/gadget/function/Makefile |   2 +
  drivers/usb/gadget/function/f_9pfs.c | 849 +++++++++++++++++++++++++++++++++++
  4 files changed, 874 insertions(+)

...

+static int alloc_requests(struct usb_composite_dev *cdev,
+			  struct f_usb9pfs *usb9pfs)
+{
+	int result = 0;
+
+	usb9pfs->in_req = usb_ep_alloc_request(usb9pfs->in_ep, GFP_ATOMIC);
+	if (!usb9pfs->in_req)
+		goto fail;
+
+	usb9pfs->out_req = usb9pfs_alloc_ep_req(usb9pfs->out_ep, usb9pfs->buflen);
+	if (!usb9pfs->out_req)
+		goto fail_in;
+
+	usb9pfs->in_req->complete = usb9pfs_tx_complete;
+	usb9pfs->out_req->complete = usb9pfs_rx_complete;
+
+	/* length will be set in complete routine */
+	usb9pfs->in_req->context = usb9pfs->out_req->context = usb9pfs;
+
+	return 0;
+
+fail_in:
+	usb_ep_free_request(usb9pfs->in_ep, usb9pfs->in_req);
+fail:
+	return result;

'result' is never overwritten, so we always return 0.
Is it on purpose?

+}

...

+static struct usb_function *usb9pfs_alloc(struct usb_function_instance *fi)
+{
+	struct f_usb9pfs_opts *usb9pfs_opts;
+	struct f_usb9pfs *usb9pfs;
+
+	usb9pfs = kzalloc(sizeof(*usb9pfs), GFP_KERNEL);
+	if (!usb9pfs)
+		return ERR_PTR(-ENOMEM);
+
+	usb9pfs_opts = container_of(fi, struct f_usb9pfs_opts, func_inst);
+
+	mutex_lock(&usb9pfs_opts->lock);
+	usb9pfs_opts->refcnt++;
+	mutex_unlock(&usb9pfs_opts->lock);
+
+	usb9pfs->buflen = usb9pfs_opts->buflen;
+
+	usb9pfs->function.name = "usb9pfs";
+	usb9pfs->function.bind = usb9pfs_func_bind;
+	usb9pfs->function.set_alt = usb9pfs_set_alt;
+	usb9pfs->function.disable = usb9pfs_disable;
+	usb9pfs->function.strings = usb9pfs_strings;
+
+	usb9pfs->function.free_func = usb9pfs_free_func;
+
+	mutex_lock(&usb9pfs_ida_lock);
+
+	usb9pfs->index = ida_simple_get(&usb9pfs_ida, 0, 100, GFP_KERNEL);

This API will soon be removed.
Please use ida_alloc_max() instead.

Also, there is no corresponding ida_free()? (or isa_simple_remove())

+	if (usb9pfs->index < 0) {
+		struct usb_function *ret = ERR_PTR(usb9pfs->index);
+
+		kfree(usb9pfs);
+		mutex_unlock(&usb9pfs_ida_lock);
+		return ret;
+	}
+
+	mutex_unlock(&usb9pfs_ida_lock);
+
+	usb9pfs->tag = kasprintf(GFP_KERNEL, "%s%d", usb9pfs->function.name,
+				 usb9pfs->index);

Same here, it seems to be freed nowhere?
I think that both should be added in usb9pfs_free_func().

CJ

+
+	INIT_LIST_HEAD(&usb9pfs->function_list);
+
+	mutex_lock(&usb9pfs_lock);
+	list_add_tail(&usb9pfs->function_list, &usbg_function_list);
+	mutex_unlock(&usb9pfs_lock);
+
+	return &usb9pfs->function;
+}

...






[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux