Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753060AbbGVVkH (ORCPT ); Wed, 22 Jul 2015 17:40:07 -0400 Received: from mail-pd0-f175.google.com ([209.85.192.175]:35071 "EHLO mail-pd0-f175.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752967AbbGVVkE (ORCPT ); Wed, 22 Jul 2015 17:40:04 -0400 Date: Wed, 22 Jul 2015 14:39:59 -0700 From: Dmitry Torokhov To: Stephen Chandler Paul Cc: Andrew Morton , Mauro Carvalho Chehab , Greg KH , Arnd Bergmann , Joe Perches , Jiri Slaby , Vishnu Patekar , Sebastian Ott , linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, linux-input@vger.kernel.org, linux-api@vger.kernel.org, Benjamin Tissoires , Hans de Goede Subject: Re: [RFC v3 1/1] Input: Add userio module Message-ID: <20150722213959.GD14875@dtor-ws> References: <1437594773-22274-1-git-send-email-cpaul@redhat.com> <1437594773-22274-2-git-send-email-cpaul@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1437594773-22274-2-git-send-email-cpaul@redhat.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2357 Lines: 88 Hi Stephen, On Wed, Jul 22, 2015 at 03:52:53PM -0400, Stephen Chandler Paul wrote: > +static int userio_char_open(struct inode *inode, struct file *file) > +{ > + struct userio_device *userio; > + > + userio = devm_kzalloc(userio_misc.this_device, > + sizeof(struct userio_device), GFP_KERNEL); > + if (!userio) > + return -ENOMEM; Never ever use devm_ API outside of device probe() calls because devm-managed resources only freed after failed probe() or after remove() callback on driver model device and not any other device (block, char, etc) in the system. > + > + spin_lock_init(&userio->head_lock); > + mutex_init(&userio->tail_lock); > + init_waitqueue_head(&userio->waitq); > + > + userio->serio = kzalloc(sizeof(struct serio), GFP_KERNEL); > + if (!userio->serio) { > + devm_kfree(userio_misc.this_device, userio); And indeed here you have ot manually release the acquired resource. So the only benefit you got form devm* is wasted resousrces. > + return -ENOMEM; > + } > + > + userio->serio->write = userio_device_write; > + userio->serio->port_data = userio; > + > + file->private_data = userio; > + > + return 0; > +} > + > +static int userio_char_release(struct inode *inode, struct file *file) > +{ > + struct userio_device *userio = file->private_data; > + > + if (userio->serio) { > + userio->serio->port_data = NULL; No need to reset, it is going away. > + > + if (userio->running) > + serio_unregister_port(userio->serio); > + else > + kfree(userio->serio); > + } > + > + devm_kfree(userio_misc.this_device, userio); > + > + return 0; > +} > + > +static ssize_t userio_char_read(struct file *file, char __user *buffer, > + size_t count, loff_t *ppos) > +{ > + struct userio_device *userio = file->private_data; > + int ret; > + size_t nonwrap_len, copylen; > + > + if (!count) > + return 0; > + > + if (file->f_flags & O_NONBLOCK && userio->head == userio->tail) > + return -EAGAIN; > + else { This is racy. If there was data and other thread "stole" it here then your O_NONBLOCK will block. See evdev_read() how you supposed to handle this. Thanks. -- Dmitry -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/