Re: [GIT PULL] USB fixes for v4.0-rc3

2015-03-14 Thread Greg KH
On Wed, Mar 11, 2015 at 11:01:07AM -0500, Felipe Balbi wrote:
> Hi Greg,
> 
> This is likely my last pull for the -rc, but things may
> change. Let me know if you need any changes
> 
> Patches tested on platforms I have access to.
> 
> cheers
> 
> The following changes since commit 9eccca0843205f87c00404b663188b88eb248051:
> 
>   Linux 4.0-rc3 (2015-03-08 16:09:09 -0700)
> 
> are available in the git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git 
> tags/fixes-for-v4.0-rc3

Pulled and pushed out.

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH V8 02/10] USB: f81232: implement RX bulk-in EP

2015-03-14 Thread Johan Hovold
On Thu, Feb 26, 2015 at 06:02:08PM +0800, Peter Hung wrote:
> The F81232 bulk-in is RX data + LSR channel, data format is
> [LSR+Data][LSR+Data]. , We had implemented in f81232_process_read_urb().
> 
> Signed-off-by: Peter Hung 

>  static void f81232_process_read_urb(struct urb *urb)
>  {
>   struct usb_serial_port *port = urb->context;
> - struct f81232_private *priv = usb_get_serial_port_data(port);
>   unsigned char *data = urb->transfer_buffer;
> - char tty_flag = TTY_NORMAL;
> - unsigned long flags;
> - u8 line_status;
> - int i;
> -
> - /* update line status */
> - spin_lock_irqsave(&priv->lock, flags);
> - line_status = priv->modem_status;
> - priv->modem_status &= ~UART_STATE_TRANSIENT_MASK;
> - spin_unlock_irqrestore(&priv->lock, flags);
> + char tty_flag;
> + unsigned int i;
> + u8 lsr;
>  
> - if (!urb->actual_length)
> + if ((urb->actual_length < 2) || (urb->actual_length % 2))
>   return;

Not parsing short data (e.g. not divisible by 2) is OK I guess. You
could also just discard the last odd byte, but that's up to you.

Either way, I think you should add a dev_warn here rather than just
silently drop it.

Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH V8 04/10] USB: f81232: implement read IIR/MSR with endpoint

2015-03-14 Thread Johan Hovold
On Thu, Feb 26, 2015 at 06:02:10PM +0800, Peter Hung wrote:
> The interrupt endpoint will report current IIR. If we got IIR with MSR changed
> , We will do read MSR with interrupt_work worker to do f81232_read_msr()
> function.
> 
> Signed-off-by: Peter Hung 
> ---
>  drivers/usb/serial/f81232.c | 126 
> +---
>  1 file changed, 118 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
> index cf5b902..46a81ef 100644
> --- a/drivers/usb/serial/f81232.c
> +++ b/drivers/usb/serial/f81232.c
> @@ -31,6 +31,13 @@ static const struct usb_device_id id_table[] = {
>  };
>  MODULE_DEVICE_TABLE(usb, id_table);
>  
> +/* USB Control EP parameter */
> +#define F81232_REGISTER_REQUEST  0xa0
> +#define F81232_GET_REGISTER  0xc0
> +
> +#define SERIAL_BASE_ADDRESS  0x0120
> +#define MODEM_STATUS_REGISTER(0x06 + SERIAL_BASE_ADDRESS)

Could you indent the values using two tabs so the
INTERRUPT_ENABLE_REGISTER in a following patch will also fit?

> +
>  #define CONTROL_DTR  0x01
>  #define CONTROL_RTS  0x02
>  
> @@ -49,19 +56,112 @@ struct f81232_private {
>   struct mutex lock;
>   u8 line_control;
>   u8 modem_status;
> + struct work_struct interrupt_work;
> + struct usb_serial_port *port;
>  };
>  
> +static int f81232_get_register(struct usb_serial_port *port, u16 reg, u8 
> *val)
> +{
> + int status;
> + u8 *tmp;
> + struct usb_device *dev = port->serial->dev;
> +
> + tmp = kmalloc(sizeof(*val), GFP_KERNEL);
> + if (!tmp)
> + return -ENOMEM;
> +
> + status = usb_control_msg(dev,
> + usb_rcvctrlpipe(dev, 0),
> + F81232_REGISTER_REQUEST,
> + F81232_GET_REGISTER,
> + reg,
> + 0,
> + tmp,
> + sizeof(u8),

Please use sizeof(*val) here as well for consistency,

> + USB_CTRL_GET_TIMEOUT);
> + if (status != sizeof(*val)) {
> + dev_err(&port->dev, "%s failed status: %d\n", __func__, status);
> +
> + if (status == 0)
> + status = -EIO;
> + else
> + status = usb_translate_errors(status);

Could you rewrite this as

if (status < 0)
status = usb_translate_errors(status);
else
status = 0;

> + } else {
> + status = 0;
> + *val = *tmp;
> + }
> +
> + kfree(tmp);
> + return status;
> +}

Same comments apply to set_register in a follow up patch.

Looks good otherwise,
Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH V8 05/10] USB: f81232: implement MCR/MSR function

2015-03-14 Thread Johan Hovold
On Thu, Feb 26, 2015 at 06:02:11PM +0800, Peter Hung wrote:
> This patch implement relative MCR/MSR function, such like
> tiocmget()/tiocmset()/dtr_rts()/carrier_raised()
> 
> original f81232_carrier_raised() compared with wrong value UART_DCD.
> It's should compared with UART_MSR_DCD.
> 
> Signed-off-by: Peter Hung 
> ---
>  drivers/usb/serial/f81232.c | 139 
> +---
>  1 file changed, 117 insertions(+), 22 deletions(-)

> +static int f81232_set_mctrl(struct usb_serial_port *port,
> +unsigned int set, unsigned int clear)
> +{
> + u8 urb_value;

Minor nit: could you just call this "val" as it's unrelated to any urb.

Also looks good otherwise.

Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH V8 07/10] USB: f81232: implement set_termios()

2015-03-14 Thread Johan Hovold
On Thu, Feb 26, 2015 at 06:02:13PM +0800, Peter Hung wrote:

> f81232_set_baudrate() is also changed from V7. Add error handling when LCR get
> failed or set LCR UART_LCR_DLAB failed. and older version, divisor is declared
> with u8, it's will make failed with baudrate lower than 600 (115200/300=384).
> We had changed divisor to int type.

No need to include this patch-revision changelog in the commit message.
You put it under the cut-off-line (---) below though.

> Signed-off-by: Peter Hung 
> ---
>  drivers/usb/serial/f81232.c | 112 
> ++--
>  1 file changed, 108 insertions(+), 4 deletions(-)

>  static int f81232_port_enable(struct usb_serial_port *port)
>  {
>   u8 val;
> @@ -391,15 +448,62 @@ static int f81232_port_disable(struct usb_serial_port 
> *port)
>  static void f81232_set_termios(struct tty_struct *tty,
>   struct usb_serial_port *port, struct ktermios *old_termios)
>  {
> - /* FIXME - Stubbed out for now */
> + u8 new_lcr = 0;
> + int status = 0;
> + speed_t baudrate;
>  
>   /* Don't change anything if nothing has changed */
>   if (old_termios && !tty_termios_hw_change(&tty->termios, old_termios))
>   return;
>  
> - /* Do the real work here... */
> - if (old_termios)
> - tty_termios_copy_hw(&tty->termios, old_termios);
> + if (C_BAUD(tty) == B0)
> + f81232_set_mctrl(port, 0, TIOCM_DTR | TIOCM_RTS);
> + else if (old_termios && (old_termios->c_cflag & CBAUD) == B0)
> + f81232_set_mctrl(port, TIOCM_DTR | TIOCM_RTS, 0);
> +
> + baudrate = tty_get_baud_rate(tty);
> + if (baudrate > 0) {
> + if (baudrate > F81232_MAX_BAUDRATE)
> + baudrate = F81232_MAX_BAUDRATE;
> +
> + tty_encode_baud_rate(tty, baudrate, baudrate);

You only need to update the termios baudrate in case you cannot set the
baudrate requested (e.g. > F81232_MAX_BAUDRATE).

> + f81232_set_baudrate(port, baudrate);
> + }
> +
> + if (C_PARENB(tty)) {
> + new_lcr |= UART_LCR_PARITY;
> +
> + if (!C_PARODD(tty))
> + new_lcr |= UART_LCR_EPAR;
> +
> + if (C_CMSPAR(tty))
> + new_lcr |= UART_LCR_SPAR;
> + }
> +
> + if (C_CSTOPB(tty))
> + new_lcr |= UART_LCR_STOP;
> +
> + switch (C_CSIZE(tty)) {
> + case CS5:
> + new_lcr |= UART_LCR_WLEN5;
> + break;
> + case CS6:
> + new_lcr |= UART_LCR_WLEN6;
> + break;
> + case CS7:
> + new_lcr |= UART_LCR_WLEN7;
> + break;
> + default:
> + case CS8:
> + new_lcr |= UART_LCR_WLEN8;
> + break;
> + }
> +
> + status = f81232_set_register(port, LINE_CONTROL_REGISTER, new_lcr);
> + if (status)
> + dev_err(&port->dev, "%s failed to set LCR: %d\n",
> + __func__, status);

Please add brackets around the if block.

> +
>  }
>  
>  static int f81232_tiocmget(struct tty_struct *tty)

Looks good otherwise.

Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH V8 08/10] USB: f81232: clarify f81232_ioctl() and fix

2015-03-14 Thread Johan Hovold
On Thu, Feb 26, 2015 at 06:02:14PM +0800, Peter Hung wrote:
> We extract TIOCGSERIAL section in f81232_ioctl() to f81232_get_serial_info()
> to make it clarify.
> 
> Also we fix device type from 16654 to 16550A, and set it's baud_base
> to 115200 (1.8432MHz/16).
> 
> Signed-off-by: Peter Hung 
> ---
>  drivers/usb/serial/f81232.c | 30 +++---
>  1 file changed, 19 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
> index 0580195..21f2342 100644
> --- a/drivers/usb/serial/f81232.c
> +++ b/drivers/usb/serial/f81232.c
> @@ -596,24 +596,32 @@ static int f81232_carrier_raised(struct usb_serial_port 
> *port)
>   return 0;
>  }
>  
> +static int f81232_get_serial_info(struct usb_serial_port *port,
> + unsigned long arg)
> +{
> + struct serial_struct ser;
> +
> + memset(&ser, 0, sizeof(ser));
> +
> + ser.type = PORT_16550A;
> + ser.line = port->minor;
> + ser.port = port->port_number;
> + ser.baud_base = 115200;

Do you want to use you MAX_BAUDRATE define here as well?

Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH V8 00/10] USB: f81232: V8 patches

2015-03-14 Thread Johan Hovold
On Thu, Mar 05, 2015 at 08:03:59AM +0100, Johan Hovold wrote:
> On Thu, Mar 05, 2015 at 01:59:45PM +0800, Peter Hung wrote:
> 
> > Did you received the series of F81232 V8 patches ? Please tell me if
> > not received.
> 
> I got it. I just haven't had time to look at it yet. Will try to do so
> this week.

I've commented on v8 now --sorry for the delay. Series looks really good
now, just a few really minor issues left.

Thanks for doing this work!

Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/5] can: kvaser_usb: Read all messages in a bulk-in URB buffer

2015-03-14 Thread Marc Kleine-Budde
On 02/26/2015 04:22 PM, Ahmed S. Darwish wrote:
> From: Ahmed S. Darwish 
> 
> The Kvaser firmware can only read and write messages that are
> not crossing the USB endpoint's wMaxPacketSize boundary. While
> receiving commands from the CAN device, if the next command in
> the same URB buffer crossed that max packet size boundary, the
> firmware puts a zero-length placeholder command in its place
> then moves the real command to the next boundary mark.
> 
> The driver did not recognize such behavior, leading to missing
> a good number of rx events during a heavy rx load session.
> 
> Moreover, a tx URB context only gets freed upon receiving its
> respective tx ACK event. Over time, the free tx URB contexts
> pool gets depleted due to the missing ACK events. Consequently,
> the netif transmission queue gets __permanently__ stopped; no
> frames could be sent again except after restarting the CAN
> newtwork interface.
> 
> Signed-off-by: Ahmed S. Darwish 

Can you please send a fix for this endianess errors (against
linux-can-fixes-for-4.0-20150314):

> drivers/net/can/usb/kvaser_usb.c:595:39: warning: restricted __le16 degrades 
> to integer
> drivers/net/can/usb/kvaser_usb.c:1332:31: warning: restricted __le16 degrades 
> to integer

Marc
-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature


Re: USB-managed UPS disconnects periodically with error "frame counter not updating; disabled"

2015-03-14 Thread Alan Stern
On Sat, 14 Mar 2015, Louis Sautier wrote:

> Hello, I am running into an issue with my USB UPS. The symptoms are
> exactly the same as the ones described by this user:
> http://comments.gmane.org/gmane.comp.monitoring.nut.user/
> I'll still describe what I experience: my UPS stops responding
> periodically (as in approximately once or twice a day but it seems to
> vary a lot). Running upsc returns "Error: Data stale"
> The kernel logs contain the following lines:
> [686576.884561] ohci-pci :00:13.0: frame counter not updating; disabled
> [686576.884564] ohci-pci :00:13.0: HC died; cleaning up
> [686576.884611] usb 5-2: USB disconnect, device number 2
> Unplugging and plugging the UPS again does nothing. I managed to get
> it working again by unbinding and binding it:
> echo :00:13.0 | tee /sys/bus/pci/drivers/ohci-pci/unbind
> /sys/bus/pci/drivers/ohci-pci/bind
> Nut's developer seems to think that this might be a kernel issue so I
> thought I would ask for advice here.
> The UPS is the only device connected to the bus:
> $ lsusb -s 05:
> Bus 005 Device 002: ID 0463: MGE UPS Systems UPS
> Bus 005 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
> I am running an x86_64 3.19.0-aufs kernel on an AMD FX(tm)-8320, nut
> 2.7.2, libusb 1.0.19 and libusb-compat 0.1.5 (I don't know which of
> the two is actually used).
> I attached my kernel config, let me know if you need anything else.

Those kernel log messages indicate that your USB controller has a
hardware problem.  Like the message says, the controller has stopped
working.  When you unbind the controller and rebind it, that performs a
reset and the controller starts working again.

If you want, you could avoid using the bad controller by getting a 
USB-2.0 hub and plugging the UPS into it.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 01/19] usb: dwc2: move debugfs code to a separate file

2015-03-14 Thread Felipe Balbi
On Mon, Mar 09, 2015 at 06:42:36PM -0700, John Youn wrote:
> On 3/9/2015 8:04 AM, Mian Yousaf Kaukab wrote:
> > +
> > +int dwc2_debugfs_init(struct dwc2_hsotg *hsotg)
> > +{
> > +   int ret;
> > +
> > +   hsotg->debug_root = debugfs_create_dir(dev_name(hsotg->dev), NULL);
> > +   if (!hsotg->debug_root) {
> > +   ret = -ENOMEM;
> > +   goto err0;
> > +   }
> > +
> > +   /* Add gadget debugfs nodes */
> > +   s3c_hsotg_create_debug(hsotg);
> > +err0:
> > +   return ret;
> > +}
> 
> Need export for this function when dwc2-platform is configured as a module.

the file is still part of the same binary, right ? EXPORT_SYMBOL*() are
only needed when functions are exposed to other modules. Usually,
EXPORT_SYMBOL*() in a driver (not in the framework) is an indication
that something's wrong ;-)

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH 02/19] usb: dwc2: debugfs: add support for complete register dump

2015-03-14 Thread Felipe Balbi
On Mon, Mar 09, 2015 at 04:04:42PM +0100, Mian Yousaf Kaukab wrote:
> Dump all registers to take a complete snapshot of dwc2 state.
> Code is inspired by dwc3/debugfs.c
> 
> Signed-off-by: Mian Yousaf Kaukab 
> ---
>  drivers/usb/dwc2/core.h|   1 +
>  drivers/usb/dwc2/debugfs.c | 356 
> +
>  2 files changed, 357 insertions(+)
> 
> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
> index 9e2bf4d..e60655f 100644
> --- a/drivers/usb/dwc2/core.h
> +++ b/drivers/usb/dwc2/core.h
> @@ -613,6 +613,7 @@ struct dwc2_hsotg {
>   enum dwc2_lx_state lx_state;
>  
>   struct dentry *debug_root;
> + struct debugfs_regset32 *regset;
>  
>   /* DWC OTG HW Release versions */
>  #define DWC2_CORE_REV_2_71a  0x4f54271a
> diff --git a/drivers/usb/dwc2/debugfs.c b/drivers/usb/dwc2/debugfs.c
> index 6c3225c..b8b105e 100644
> --- a/drivers/usb/dwc2/debugfs.c
> +++ b/drivers/usb/dwc2/debugfs.c
> @@ -391,9 +391,344 @@ static inline void s3c_hsotg_create_debug(struct 
> dwc2_hsotg *hsotg) {}
>  
>  /* s3c_hsotg_delete_debug is removed as cleanup in done in dwc2_debugfs_exit 
> */
>  
> +#define dump_register(nm)\
> +{\
> + .name   = #nm,  \

sure you don't need __stringify() here ?

-- 
balbi


signature.asc
Description: Digital signature


Re: USB-managed UPS disconnects periodically with error "frame counter not updating; disabled"

2015-03-14 Thread Louis Sautier
On 14 March 2015 at 16:29, Alan Stern  wrote:
> On Sat, 14 Mar 2015, Louis Sautier wrote:
>
>> Hello, I am running into an issue with my USB UPS. The symptoms are
>> exactly the same as the ones described by this user:
>> http://comments.gmane.org/gmane.comp.monitoring.nut.user/
>> I'll still describe what I experience: my UPS stops responding
>> periodically (as in approximately once or twice a day but it seems to
>> vary a lot). Running upsc returns "Error: Data stale"
>> The kernel logs contain the following lines:
>> [686576.884561] ohci-pci :00:13.0: frame counter not updating; disabled
>> [686576.884564] ohci-pci :00:13.0: HC died; cleaning up
>> [686576.884611] usb 5-2: USB disconnect, device number 2
>> Unplugging and plugging the UPS again does nothing. I managed to get
>> it working again by unbinding and binding it:
>> echo :00:13.0 | tee /sys/bus/pci/drivers/ohci-pci/unbind
>> /sys/bus/pci/drivers/ohci-pci/bind
>> Nut's developer seems to think that this might be a kernel issue so I
>> thought I would ask for advice here.
>> The UPS is the only device connected to the bus:
>> $ lsusb -s 05:
>> Bus 005 Device 002: ID 0463: MGE UPS Systems UPS
>> Bus 005 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
>> I am running an x86_64 3.19.0-aufs kernel on an AMD FX(tm)-8320, nut
>> 2.7.2, libusb 1.0.19 and libusb-compat 0.1.5 (I don't know which of
>> the two is actually used).
>> I attached my kernel config, let me know if you need anything else.
>
> Those kernel log messages indicate that your USB controller has a
> hardware problem.  Like the message says, the controller has stopped
> working.  When you unbind the controller and rebind it, that performs a
> reset and the controller starts working again.
>
> If you want, you could avoid using the bad controller by getting a
> USB-2.0 hub and plugging the UPS into it.
>
> Alan Stern
>
So this is definitely a hardware problem? I updated my motherboard
BIOS just to be sure.
Would using some ports from another controller (maybe USB2/USB3 ones)
change anything?
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: USB-managed UPS disconnects periodically with error "frame counter not updating; disabled"

2015-03-14 Thread Alan Stern
On Sat, 14 Mar 2015, Louis Sautier wrote:

> > Those kernel log messages indicate that your USB controller has a
> > hardware problem.  Like the message says, the controller has stopped
> > working.  When you unbind the controller and rebind it, that performs a
> > reset and the controller starts working again.
> >
> > If you want, you could avoid using the bad controller by getting a
> > USB-2.0 hub and plugging the UPS into it.
> >
> > Alan Stern
> >
> > So this is definitely a hardware problem? I updated my motherboard BIOS
> just to be sure.

I'm pretty sure it's a hardware problem.

> Would using some ports from another controller (maybe USB2/USB3 ones)
> change anything?

Yes, it would.  That's why I suggested you get a hub.  Getting a PCI 
card with a USB controller would also work.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] phy: Add a driver for dm816x USB PHY

2015-03-14 Thread Matthijs van Duin
On 13 March 2015 at 20:30, Tony Lindgren  wrote:
> Hmm OK have to check that. It could also be that dm816x documentation
> is copy-paste from da850 or am3517 and the PHY got changed in the
> hardware as the registers don't match the documentation. Only the
> dm816x errata has right documentation for the USB PHY.

Hmm? While I see plenty of usb errata (mostly DMA bugs), I don't see
anything about registers being different.

I do see something curious: advisory 70, the only PHY-related erratum
I see, is also present in the DM814x errata and even in AM335x r1.0.
This strongly suggests the PHYs must at least be closely related...

The dm816x TRM makes three separate mentions of the synopsys usb phy
though, while I found no other TRMs that mention it, so if it's a
copy-paste error (which certainly would not be exceptional) I don't
know where from.

I suppose it's still possible TI acquired a sufficiently permissive
license for the synopsys phy to fork it and call it a "TI PHY" as they
do in the AM335x docs. (No mention of its origin is made in the DM814x
docs.)

BTW, da850? Is that yet another instance of Primus? (i.e.
omap-L1xx/c674x/am1xxx with odd final digit, also da830/da828)

Matthijs
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [git pull] gadgetfs fixes

2015-03-14 Thread Alexander Holler
Am 13.03.2015 um 17:42 schrieb Al Viro:
>   Assorted fixes around AIO on gadgetfs: leaks, use-after-free,
> troubles caused by ->f_op flipping.  Please, pull from
> git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git gadget
> 
> Shortlog:
> Al Viro (8):
>   new helper: dup_iter()
>   move iov_iter.c from mm/ to lib/
>   gadget/function/f_fs.c: close leaks
>   gadget/function/f_fs.c: use put iov_iter into io_data
>   gadget/function/f_fs.c: switch to ->{read,write}_iter()

>   gadgetfs: use-after-free in ->aio_read()

If that patch ends up in the stable kernels (as it is marked as such),
it needs a
value = -ENOMEM
before that added "goto fail;", otherwise the return value is unitialized.

Alexander Holler
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [git pull] gadgetfs fixes

2015-03-14 Thread Al Viro
On Sun, Mar 15, 2015 at 01:39:21AM +0100, Alexander Holler wrote:
> Am 13.03.2015 um 17:42 schrieb Al Viro:
> > Assorted fixes around AIO on gadgetfs: leaks, use-after-free,
> > troubles caused by ->f_op flipping.  Please, pull from
> > git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git gadget
> > 
> > Shortlog:
> > Al Viro (8):
> >   new helper: dup_iter()
> >   move iov_iter.c from mm/ to lib/
> >   gadget/function/f_fs.c: close leaks
> >   gadget/function/f_fs.c: use put iov_iter into io_data
> >   gadget/function/f_fs.c: switch to ->{read,write}_iter()
> 
> >   gadgetfs: use-after-free in ->aio_read()
> 
> If that patch ends up in the stable kernels (as it is marked as such),
> it needs a
>   value = -ENOMEM
> before that added "goto fail;", otherwise the return value is unitialized.

Umm...  If I'm not misparsing what you said, you are talking about the
one that gets removed by
-   if (iv) {
-   priv->iv = kmemdup(iv, nr_segs * sizeof(struct iovec),
-  GFP_KERNEL);
-   if (!priv->iv) {
-   kfree(priv);
-   goto fail;
-   }
-   }
in "gadget: switch ep_io_operations to ->read_iter/->write_iter" very
shortly afterwards, and _that_ is a prereq for ->f_op flipping fixes,
which is also clear -stable fodder.  But yes, it's a bisect hazard and
a cherry-pick one as well.  Nice catch...
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [git pull] gadgetfs fixes

2015-03-14 Thread Alexander Holler
Am 15.03.2015 um 02:39 schrieb Al Viro:
> On Sun, Mar 15, 2015 at 01:39:21AM +0100, Alexander Holler wrote:
>> Am 13.03.2015 um 17:42 schrieb Al Viro:
>>> Assorted fixes around AIO on gadgetfs: leaks, use-after-free,
>>> troubles caused by ->f_op flipping.  Please, pull from
>>> git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git gadget
>>>
>>> Shortlog:
>>> Al Viro (8):
>>>   new helper: dup_iter()
>>>   move iov_iter.c from mm/ to lib/
>>>   gadget/function/f_fs.c: close leaks
>>>   gadget/function/f_fs.c: use put iov_iter into io_data
>>>   gadget/function/f_fs.c: switch to ->{read,write}_iter()
>>
>>>   gadgetfs: use-after-free in ->aio_read()
>>
>> If that patch ends up in the stable kernels (as it is marked as such),
>> it needs a
>>  value = -ENOMEM
>> before that added "goto fail;", otherwise the return value is unitialized.
> 
> Umm...  If I'm not misparsing what you said, you are talking about the

Glücklicherweise nicht. Vielleicht sollten wir es zur Abwechslung mal
mit meiner bevorzugten Sprache versuchen.

> one that gets removed by
> -   if (iv) {
> -   priv->iv = kmemdup(iv, nr_segs * sizeof(struct iovec),
> -  GFP_KERNEL);
> -   if (!priv->iv) {
> -   kfree(priv);
> -   goto fail;
> -   }
> -   }
> in "gadget: switch ep_io_operations to ->read_iter/->write_iter" very
> shortly afterwards, and _that_ is a prereq for ->f_op flipping fixes,
> which is also clear -stable fodder.  But yes, it's a bisect hazard and
> a cherry-pick one as well.  Nice catch...

The following patches aren't marked for stable, otherwise I would not
have risked to become a victim of your comments again.

Alexander Holler
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html