From patchwork Wed Nov 17 05:20:15 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dan Carpenter X-Patchwork-Id: 4927 Return-path: Envelope-to: mchehab@pedra Delivery-date: Wed, 17 Nov 2010 09:39:37 -0200 Received: from mchehab by pedra with local (Exim 4.72) (envelope-from ) id 1PIgMS-00012z-Ms for mchehab@pedra; Wed, 17 Nov 2010 09:39:37 -0200 Received: from casper.infradead.org [85.118.1.10] by pedra with IMAP (fetchmail-6.3.17) for (single-drop); Wed, 17 Nov 2010 09:39:36 -0200 (BRST) Received: from vger.kernel.org ([209.132.180.67]) by casper.infradead.org with esmtp (Exim 4.72 #1 (Red Hat Linux)) id 1PIaSD-00041U-41; Wed, 17 Nov 2010 05:21:09 +0000 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752103Ab0KQFVG (ORCPT + 1 other); Wed, 17 Nov 2010 00:21:06 -0500 Received: from mail-wy0-f174.google.com ([74.125.82.174]:49323 "EHLO mail-wy0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751718Ab0KQFVF (ORCPT ); Wed, 17 Nov 2010 00:21:05 -0500 Received: by wyb28 with SMTP id 28so1589513wyb.19 for ; Tue, 16 Nov 2010 21:21:03 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:date:from:to:cc:subject :message-id:mime-version:content-type:content-disposition:user-agent; bh=cY4UbRN1+zEmuo7J1/lBmSrAMvLnBRRI87Qlpf7ruuw=; b=vZg9tMQAqbdpKvvhp7LsLoVyTZR2drEdPCxqPmuWJlZMNRz09T8W6I/2CftkGGJY9W XVH4YSeDD+oJ6fp2+Z4EFXuPEjeAGTeDaka8Q+oGuL3uBWIEU0h2Mh032RXPlJfSJO3+ 3UbzACNB19to2jnyhTrKJXi4++dbLOA4vnoqo= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=date:from:to:cc:subject:message-id:mime-version:content-type :content-disposition:user-agent; b=qdLBy2dEMq4Ly7W1Hyc4wMcUnEFjh61wJnT1s4q1zS8dSKPVxYZH9R/6oOaBrVwyFz m3YjiXUuwb8Om0ngDln9c1VaL1bM5vKcKBE79kaqaz1lPRWd/VUTzmV2aO4l+S2sq85V uMthW+Qb2kBNHz9oohrjXOB58/9Kas3tWpQng= Received: by 10.216.30.2 with SMTP id j2mr9165858wea.33.1289971262545; Tue, 16 Nov 2010 21:21:02 -0800 (PST) Received: from bicker (h14ba.n2.ips.mtn.co.ug [212.88.116.186]) by mx.google.com with ESMTPS id w84sm939245weq.44.2010.11.16.21.20.39 (version=TLSv1/SSLv3 cipher=RC4-MD5); Tue, 16 Nov 2010 21:21:01 -0800 (PST) Date: Wed, 17 Nov 2010 08:20:15 +0300 From: Dan Carpenter To: Mauro Carvalho Chehab Cc: Jarod Wilson , Zimny Lech , linux-media@vger.kernel.org, kernel-janitors@vger.kernel.org Subject: [patch 3/3] [media] lirc_dev: fixes in lirc_dev_fop_read() Message-ID: <20101117052015.GF31724@bicker> MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.5.20 (2009-06-14) Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org Sender: This makes several changes but they're in one function and sort of related: "buf" was leaked on error. The leak if we try to read an invalid length is the main concern because it could be triggered over and over. If the copy_to_user() failed, then the original code returned the number of bytes remaining. read() is supposed to be the opposite way, where we return the number of bytes copied. I changed it to just return -EFAULT on errors. Also I changed the debug output from "-EFAULT" to just "" because it isn't -EFAULT necessarily. And since we go though that path if the length is invalid now, there was another debug print that I removed. Signed-off-by: Dan Carpenter Reviewed-by: Jarod Wilson Acked-by: Jarod Wilson --- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/media/IR/lirc_dev.c b/drivers/media/IR/lirc_dev.c index fbca94f..6b9fc74 100644 --- a/drivers/media/IR/lirc_dev.c +++ b/drivers/media/IR/lirc_dev.c @@ -647,18 +647,18 @@ ssize_t lirc_dev_fop_read(struct file *file, if (!buf) return -ENOMEM; - if (mutex_lock_interruptible(&ir->irctl_lock)) - return -ERESTARTSYS; + if (mutex_lock_interruptible(&ir->irctl_lock)) { + ret = -ERESTARTSYS; + goto out_unlocked; + } if (!ir->attached) { - mutex_unlock(&ir->irctl_lock); - return -ENODEV; + ret = -ENODEV; + goto out_locked; } if (length % ir->chunk_size) { - dev_dbg(ir->d.dev, LOGHEAD "read result = -EINVAL\n", - ir->d.name, ir->d.minor); - mutex_unlock(&ir->irctl_lock); - return -EINVAL; + ret = -EINVAL; + goto out_locked; } /* @@ -709,18 +709,23 @@ ssize_t lirc_dev_fop_read(struct file *file, lirc_buffer_read(ir->buf, buf); ret = copy_to_user((void *)buffer+written, buf, ir->buf->chunk_size); - written += ir->buf->chunk_size; + if (!ret) + written += ir->buf->chunk_size; + else + ret = -EFAULT; } } remove_wait_queue(&ir->buf->wait_poll, &wait); set_current_state(TASK_RUNNING); + +out_locked: mutex_unlock(&ir->irctl_lock); out_unlocked: kfree(buf); dev_dbg(ir->d.dev, LOGHEAD "read result = %s (%d)\n", - ir->d.name, ir->d.minor, ret ? "-EFAULT" : "OK", ret); + ir->d.name, ir->d.minor, ret ? "" : "", ret); return ret ? ret : written; }