From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from fraxinus.osuosl.org (fraxinus.osuosl.org [140.211.166.137]) by ash.osuosl.org (Postfix) with ESMTP id EE80F1BFA19 for ; Tue, 8 Jul 2014 17:52:05 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id EA92C88071 for ; Tue, 8 Jul 2014 17:52:05 +0000 (UTC) Received: from fraxinus.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id hqXaUj72X9GO for ; Tue, 8 Jul 2014 17:52:04 +0000 (UTC) Received: from aserp1040.oracle.com (aserp1040.oracle.com [141.146.126.69]) by fraxinus.osuosl.org (Postfix) with ESMTPS id D49058801F for ; Tue, 8 Jul 2014 17:52:04 +0000 (UTC) Date: Tue, 8 Jul 2014 20:52:00 +0300 From: Dan Carpenter Subject: Re: [PATCH 2/2] staging: unisys: added virtpci info entry Message-ID: <20140708175159.GS25880@mwanda> References: <1404840091-12058-1-git-send-email-erik.arfvidson@unisys.com> <1404840091-12058-2-git-send-email-erik.arfvidson@unisys.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1404840091-12058-2-git-send-email-erik.arfvidson@unisys.com> List-Id: Linux Driver Project Developer List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: driverdev-devel-bounces@linuxdriverproject.org Sender: driverdev-devel-bounces@linuxdriverproject.org To: Erik Arfvidson Cc: gregkh@linuxfoundation.org, benjamin.romer@unisys.com, sparmaintainer@unisys.com, driverdev-devel@linuxdriverproject.org On Tue, Jul 08, 2014 at 01:21:31PM -0400, Erik Arfvidson wrote: > This patch adds the virtpci debugfs directory and the info entry > inside of it. > > Signed-off-by: Erik Arfvidson > Signed-off-by: Benjamin Romer > --- > drivers/staging/unisys/virtpci/virtpci.c | 122 ++++++++++++++++++++++++++++++- > 1 file changed, 118 insertions(+), 4 deletions(-) > > diff --git a/drivers/staging/unisys/virtpci/virtpci.c b/drivers/staging/unisys/virtpci/virtpci.c > index 7d840b0..6c0aa96 100644 > --- a/drivers/staging/unisys/virtpci/virtpci.c > +++ b/drivers/staging/unisys/virtpci/virtpci.c > @@ -37,6 +37,7 @@ > #include > #include > #include "version.h" > + #include > #include "guestlinuxdebug.h" > #include "timskmodutils.h" > > @@ -100,6 +101,12 @@ static int virtpci_device_resume(struct device *dev); > static int virtpci_device_probe(struct device *dev); > static int virtpci_device_remove(struct device *dev); > > +static ssize_t info_debugfs_read(struct file *file, char __user *buf, > + size_t len, loff_t *offset); > + > +static const struct file_operations debugfs_info_fops = { > + .read = info_debugfs_read, > +}; > > /*****************************************************/ > /* Globals */ > @@ -139,7 +146,19 @@ static DEFINE_RWLOCK(VpcidevListLock); > > /* filled in with info about this driver, wrt it servicing client busses */ > static ULTRA_VBUS_DEVICEINFO Bus_DriverInfo; > - > + /*****************************************************/ > + /* DebugFS entries */ > + /*****************************************************/ > +/* dentry is used to create the debugfs entry directory > + * for virtpci > + */ > +static struct dentry *virtpci_debugfs_dir; > +static struct dentry *info_debugfs_entry; > +/* info_debugfs_entry is used to tell virtpci to display current info > + * kept in the driver > + */ > +#define DIR_DEBUGFS_ENTRY "virtpci" > +#define INFO_DEBUGFS_ENTRY_FN "info" > > struct virtpci_busdev { > struct device virtpci_bus_device; > @@ -588,7 +607,8 @@ static void delete_all(void) > /* deletes all vnics or vhbas > * returns 0 failure, 1 success, > */ > -static int delete_all_virt(VIRTPCI_DEV_TYPE devtype, struct del_vbus_guestpart *delparams) > +static int delete_all_virt(VIRTPCI_DEV_TYPE devtype, > + struct del_vbus_guestpart *delparams) > { > int i; > unsigned char busid[BUS_ID_SIZE]; > @@ -1375,6 +1395,95 @@ void virtpci_unregister_driver(struct virtpci_driver *drv) > DBGINF("Leaving\n"); > } > EXPORT_SYMBOL_GPL(virtpci_unregister_driver); > +/*****************************************************/ > +/* debugfs filesystem functions */ > +/*****************************************************/ > +struct print_vbus_info { > + int *length; > + char *buf; > +}; > + > +static int print_vbus(struct device *vbus, void *data) > +{ > + struct print_vbus_info *p = (struct print_vbus_info *) data; Remove the space after the cast to remind that casting is high precedence. struct print_vbus_info *p = (struct print_vbus_info *)data; > + int l = *(p->length); Remove the extra parens. int l = *p->length; > + > + *(p->length) = l + sprintf(p->buf + l, "bus_id:%s\n", dev_name(vbus)); You don't really need the "l" at all. *p->length += sprintf(p->buf + *p->length, "bus_id:%s\n", dev_name(vbus)); > + return 0; /* no error */ > +} > + > +static ssize_t info_debugfs_read(struct file *file, char __user *buf, > + size_t len, loff_t *offset) > +{ > + int length = 0; "length" is going to be confusing because we already have "len". > + struct virtpci_dev *tmpvpcidev; > + unsigned long flags; > + struct print_vbus_info printparam; > + char *vbuf; > + loff_t pos = *offset; > + > + if (pos < 0) > + return -EINVAL; > + > + if (pos > 0 || !len) > + return 0; I don't understand the point of updating *offset and then not allowing pos > 0. (This could be because I haven't paid attention as well though). > + > + vbuf = kzalloc(len, GFP_KERNEL); > + if (!vbuf) > + return -ENOMEM; > + > + length += sprintf(vbuf + length, "CHANSOCK is not defined.\n"); This is memory corruption here because we don't verify that len is large enough for the string. None of the sprintf() calls have checking. > + > + length += sprintf(vbuf + length, "\n Virtual PCI Bus devices\n"); > + printparam.length = &length; > + printparam.buf = vbuf; > + if (bus_for_each_dev(&virtpci_bus_type, NULL, > + (void *) &printparam, print_vbus)) > + LOGERR("delete of all vbus failed\n"); "delete"? Probably cut and paste bug. > + > + length += sprintf(vbuf + length, "\n Virtual PCI devices\n"); > + read_lock_irqsave(&VpcidevListLock, flags); > + tmpvpcidev = VpcidevListHead; > + while (tmpvpcidev) { > + if (tmpvpcidev->devtype == VIRTHBA_TYPE) { > + length += sprintf(vbuf + length, > + "[%d:%d] VHba:%08x:%08x max-config:%d-%d-%d-%d", > + tmpvpcidev->busNo, tmpvpcidev->deviceNo, > + tmpvpcidev->scsi.wwnn.wwnn1, > + tmpvpcidev->scsi.wwnn.wwnn2, > + tmpvpcidev->scsi.max.max_channel, > + tmpvpcidev->scsi.max.max_id, > + tmpvpcidev->scsi.max.max_lun, > + tmpvpcidev->scsi.max.cmd_per_lun); > + } else { > + length += sprintf(vbuf + length, "[%d:%d] VNic:%02x:%02x:%02x:%02x:%02x:%02x num_rcv_bufs:%d mtu:%d", > + tmpvpcidev->busNo, tmpvpcidev->deviceNo, > + tmpvpcidev->net.mac_addr[0], > + tmpvpcidev->net.mac_addr[1], > + tmpvpcidev->net.mac_addr[2], > + tmpvpcidev->net.mac_addr[3], > + tmpvpcidev->net.mac_addr[4], > + tmpvpcidev->net.mac_addr[5], > + tmpvpcidev->net.num_rcv_bufs, > + tmpvpcidev->net.mtu); > + } > + length += > + sprintf(vbuf + length, " chanptr:%p\n", > + tmpvpcidev->queueinfo.chan); > + tmpvpcidev = tmpvpcidev->next; > + } > + read_unlock_irqrestore(&VpcidevListLock, flags); > + > + length += sprintf(vbuf + length, "\n"); > + if (copy_to_user(buf, vbuf, length)) { There should be some length checking here as well. > + kfree(vbuf); > + return -EFAULT; > + } > + > + kfree(vbuf); > + *offset += length; > + return length; > +} > > /*****************************************************/ > /* Module Init & Exit functions */ > @@ -1426,7 +1535,10 @@ static int __init virtpci_mod_init(void) > > LOGINF("successfully registered virtpci_ctrlchan_func (0x%p) as callback.\n", > (void *) &virtpci_ctrlchan_func); > - > + /*create debugfs directory*/ > + virtpci_debugfs_dir = debugfs_create_dir(DIR_DEBUGFS_ENTRY, NULL); > + info_debugfs_entry = debugfs_create_file(INFO_DEBUGFS_ENTRY_FN, 0660, The owner has write permision but there is no ->write op. Don't use the numbers use S_IRUSR or whatever. > + virtpci_debugfs_dir, NULL, &debugfs_info_fops); > LOGINF("Leaving\n"); > POSTCODE_LINUX_2(VPCI_CREATE_EXIT_PC, POSTCODE_SEVERITY_INFO); > return 0; > @@ -1442,7 +1554,9 @@ static void __exit virtpci_mod_exit(void) > > device_unregister(&virtpci_rootbus_device); > bus_unregister(&virtpci_bus_type); > - > + /*debug_fs file and direcroty removal*/ > + debugfs_remove(info_debugfs_entry); > + debugfs_remove(virtpci_debugfs_dir); These are not indented correctly. regards, dan carpenter _______________________________________________ devel mailing list devel@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel