linux-kernel - RE: [RFC PATCH 02/06] input/rmi4: Core files

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <BFABBC009FBA1C43B207B15FD485ABD922C22B7B@USW-MAIL1.synaptics-inc.local>
Date:	Thu, 11 Oct 2012 02:49:02 +0000
From:	Christopher Heiny <Cheiny@...aptics.com>
To:	Joe Perches <joe@...ches.com>
CC:	Dmitry Torokhov <dmitry.torokhov@...il.com>,
	Jean Delvare <khali@...ux-fr.org>,
	Linux Kernel <linux-kernel@...r.kernel.org>,
	Linux Input <linux-input@...r.kernel.org>,
	Allie Xiong <axiong@...aptics.com>,
	Vivian Ly <vly@...aptics.com>,
	Daniel Rosenberg <daniel.rosenberg@...aptics.com>,
	Joerie de Gram <j.de.gram@...il.com>,
	Wolfram Sang <w.sang@...gutronix.de>,
	Mathieu Poirier <mathieu.poirier@...aro.org>,
	Linus Walleij <linus.walleij@...ricsson.com>,
	Naveen Kumar Gaddipati <naveen.gaddipati@...ricsson.com>,
	Alexandra Chin <alexandra.chin@...synaptics.com>
Subject: RE: [RFC PATCH 02/06] input/rmi4: Core files

Joe Perches wrote:
> On Fri, 2012-10-05 at 21:09 -0700, Christopher Heiny wrote:
> []
> 
> Just some trivial comments:

Thanks - see below for responses.

> > diff --git a/drivers/input/rmi4/rmi_driver.c
> > b/drivers/input/rmi4/rmi_driver.c
> []
> 
> > @@ -0,0 +1,1529 @@
> 
> []
> 
> > +static ssize_t delay_write(struct file *filp, const char __user *buffer,
> > +                        size_t size, loff_t *offset) {
> > +     struct driver_debugfs_data *data = filp->private_data;
> > +     struct rmi_device_platform_data *pdata =
> > +                     data->rmi_dev->phys->dev->platform_data;
> > +     int retval;
> > +     char local_buf[size];
> > +     unsigned int new_read_delay;
> > +     unsigned int new_write_delay;
> > +     unsigned int new_block_delay;
> > +     unsigned int new_pre_delay;
> > +     unsigned int new_post_delay;
> > +
> > +     retval = copy_from_user(local_buf, buffer, size);
> > +     if (retval)
> > +             return -EFAULT;
> > +
> > +     retval = sscanf(local_buf, "%u %u %u %u %u", &new_read_delay,
> > +                     &new_write_delay, &new_block_delay,
> > +                     &new_pre_delay, &new_post_delay);
> > +     if (retval != 5) {
> > +             dev_err(&data->rmi_dev->dev,
> > +                     "Incorrect number of values provided for delay.");
> > +             return -EINVAL;
> > +     }
> > +     if (new_read_delay < 0) {
> 
> These are unnecessary tests as unsigned values are never < 0.

Right.  Thought we'd taken care of most of the silliness like that, but obviously missed some.  We'll recheck the codebase.

[snip]


> > +static ssize_t phys_read(struct file *filp, char __user *buffer, size_t
> > size, +                 loff_t *offset) {
> > +     struct driver_debugfs_data *data = filp->private_data;
> > +     struct rmi_phys_info *info = &data->rmi_dev->phys->info;
> > +     int retval;
> > +     char local_buf[size];
> 
> size comes from where?  possible stack overflow?

Hmmmm.  Good point.  We'll look at this.

[snip]

> []
> 
> > +     list_for_each_entry(entry, &data->rmi_functions.list, list)
> > +             if (entry->irq_mask)
> > +                     process_one_interrupt(entry, irq_status,
> > +                                           data);
> 
> style nit, it'd be nicer with braces.

I agree with you, but checkpatch.pl doesn't. :-(

> 
> > diff --git a/drivers/input/rmi4/rmi_driver.h
> > b/drivers/input/rmi4/rmi_driver.h
> []
> 
> > @@ -0,0 +1,438 @@
> > 
> > +
> > +#define tricat(x, y, z) tricat_(x, y, z)
> > +
> > +#define tricat_(x, y, z) x##y##z
> 
> I think these tricat macros are merely obfuscating
> and don't need to be used.

tricat is used internally by another collection of macros that helps generate sysfs files.  In particular, it's used to generate the RMI4 register name symbols.--
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/