From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756907AbcIPHIA (ORCPT ); Fri, 16 Sep 2016 03:08:00 -0400 Received: from mailout1.w1.samsung.com ([210.118.77.11]:33541 "EHLO mailout1.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755380AbcIPHHx (ORCPT ); Fri, 16 Sep 2016 03:07:53 -0400 X-AuditID: cbfec7ef-f79e76d000005b57-65-57db9a44d52c Subject: Re: [PATCH v3] leds: Introduce userspace leds driver To: Pavel Machek , David Lechner Cc: Richard Purdie , linux-kernel@vger.kernel.org, linux-leds@vger.kernel.org, Marcel Holtmann From: Jacek Anaszewski Message-id: Date: Fri, 16 Sep 2016 09:07:45 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-version: 1.0 In-reply-to: <20160916055014.GA13205@amd> Content-type: text/plain; charset=windows-1252; format=flowed Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFupgleLIzCtJLcpLzFFi42LZduznOV2XWbfDDXZskbJY1CBmcXnXHDaL rW/WMVp8+/SL0eLuqaNsFrt3PWV1YPNYv3s5u8en/pOsHnvm/2D1WLH6O7vH501yAaxRXDYp qTmZZalF+nYJXBnfH91mKdjKV/H0/AfGBsYPXF2MnBwSAiYSux6fYIGwxSQu3FvP1sXIxSEk sIxRYsaGPewQzmdGibUzDrLCdJx+vIoZrurtk13MIAkhgWeMEufPiYPYwgK2Eus2XmPqYuTg EBFwlVg+qRwkzCzQyShx8ngJiM0mYCjx88VrsBJeATuJizdkQMIsAqoS1xbcYgEJiwpESOy+ mwoS5hUQlPgx+R7YnZwCmhJnenaxQEx0lHiwaCcrhC0vsXnNW7DLJATmsUvM+zsdbLyEgKzE pgPMEKaLxK7zQhCPCEu8Or6FHcKWkbg8uZsFonUyo8TFYzdZIZzVjBIbOzuhAWQt0fD/F9Ri PolJ26ZDDeWV6GiDGuoh0bLiHzOE7SjxoXUiNAh/MEm8/3ydfQKj/Cwk/8xC8sMsJD8sYGRe xSiSWlqcm55abKhXnJhbXJqXrpecn7uJEZg+Tv87/n4H49PmkEOMAhyMSjy8K+beChdiTSwr rsw9xCjBwawkwts27Xa4EG9KYmVValF+fFFpTmrxIUZpDhYlcd69C66ECwmkJ5akZqemFqQW wWSZODilGhini58Lzhctu/JVsG9CUH6zzJmo0jVz4k/tDXnWM/3ThEe/DzoWTGs8bPvrZll2 XeWu4m6nJQzPVHvynPV/bPvx+oFXtdSuDefMXy5ym/exryZFVXbh9afC2T+Dpi14W6fBfvlH e9e1Q9xXPl+ZpWSiYrKDc0rm+1WBxjpTr/S0HhY0a+oQdypTYinOSDTUYi4qTgQAKwuPaBsD AAA= X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFlrNIsWRmVeSWpSXmKPExsVy+t/xy7rCs26HGzzdomKxqEHM4vKuOWwW W9+sY7T49ukXo8XdU0fZLHbvesrqwOaxfvdydo9P/SdZPfbM/8HqsWL1d3aPz5vkAlij3Gwy UhNTUosUUvOS81My89JtlUJD3HQtlBTyEnNTbZUidH1DgpQUyhJzSoE8IwM04OAc4B6spG+X 4Jbx/dFtloKtfBVPz39gbGD8wNXFyMkhIWAicfrxKmYIW0ziwr31bF2MXBxCAksYJT49PcIO 4TxjlNi6ciITSJWwgK3Euo3XgGwODhEBV4nlk8ohan4wScxc+Z4ZxGEW6GSU6D6+nAWkgU3A UOLni9dgDbwCdhIXb8iAhFkEVCWuLbgFViIqECFxa9VHRhCbV0BQ4sfke2BxTgFNiTM9u8Bs ZqC9C96vg7LlJTavecs8gVFgFpKWWUjKZiEpW8DIvIpRJLW0ODc9t9hIrzgxt7g0L10vOT93 EyMworYd+7llB2PXu+BDjAIcjEo8vCvm3goXYk0sK67MPcQowcGsJMLbNu12uBBvSmJlVWpR fnxRaU5q8SFGU6AnJjJLiSbnA6M9ryTe0MTQ3NLQyNjCwtzISEmcd+qHK+FCAumJJanZqakF qUUwfUwcnFINjLFrJpTUKGVdk2hSa/3+4sq51wevyj7vUjFaZhV++ZVyS5KJvKrf+rbotuvz U64d6omYwjHzz1luJ2Pt4xm32D482TZzQq9+bcmhc0W12W/DtO17zytang++55y3eMm/9lln Iuofml74JOksf8R55fzjwrvPzfiYuvAPl+6x2iTlMiPHxDPJPEosxRmJhlrMRcWJAKlLdxG+ AgAA X-MTR: 20000000000000000@CPGS X-CMS-MailID: 20160916070747eucas1p1aeca6ae589707574900e618f9ccbbf40 X-Msg-Generator: CA X-Sender-IP: 182.198.249.180 X-Local-Sender: =?UTF-8?B?SmFjZWsgQW5hc3pld3NraRtTUlBPTC1TeXN0ZW0gRlcgIChN?= =?UTF-8?B?Qikb7IK87ISx7KCE7J6QG1NlbmlvciBTb2Z0d2FyZSBFbmdpbmVlcg==?= X-Global-Sender: =?UTF-8?B?SmFjZWsgQW5hc3pld3NraRtTUlBPTC1TeXN0ZW0gRlcgIChN?= =?UTF-8?B?QikbU2Ftc3VuZyBFbGVjdHJvbmljcxtTZW5pb3IgU29mdHdhcmUgRW5naW5l?= =?UTF-8?B?ZXI=?= X-Sender-Code: =?UTF-8?B?QzEwG0VIURtDMTBDRDAyQ0QwMjc1MjY=?= CMS-TYPE: 201P X-HopCount: 7 X-CMS-RootMailID: 20160912081846eucas1p255044e49034685ad44400d6830ef0b95 X-RootMTR: 20160912081846eucas1p255044e49034685ad44400d6830ef0b95 References: <1473439776-15655-1-git-send-email-david@lechnology.com> <80597ded-f4b4-2990-3eae-e72276296d1a@samsung.com> <20160915130831.GJ13132@amd> <313cbae5-fd66-f0ae-79a9-a3f4273d6f9c@samsung.com> <20160916055014.GA13205@amd> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 09/16/2016 07:50 AM, Pavel Machek wrote: > Hi! > >>>>>> + if (copy_from_user(&udev->user_dev, buffer, >>>>>> + sizeof(struct uleds_user_dev))) { >>>>>> + ret = -EFAULT; >>>>>> + goto out; >>>>>> + } >>>>>> + >>>>>> + if (!udev->user_dev.name[0]) { >>>>>> + ret = -EINVAL; >>>>>> + goto out; >>>>>> + } >>>>>> + >>>>>> + ret = led_classdev_register(NULL, &udev->led_cdev); >>>>>> + if (ret < 0) >>>>>> + goto out; >>>> >>>> No sanity checking on the name -> probably a security hole. Do not >>>> push this upstream before this is fixed. >>> >> >> If this is a serious security issue, then you should also raise an issue >> with input maintainers because this is the extent of sanity checking for >> uinput device names as well. > > I guess that should be fixed. But lets not add new ones. > >> I must confess that I am no security expert, so unless you can give specific >> examples of what potential threats are, I will not be able to guess what I >> need to do to fix it. >> >> After some digging around the kernel, I don't see many instances of >> validating device node names. The best I have found so far comes from >> create_entry() in binfmt_misc.c >> >> if (!e->name[0] || >> !strcmp(e->name, ".") || >> !strcmp(e->name, "..") || >> strchr(e->name, '/')) >> goto einval; >> >> Would something like this be a sufficient sanity check? I suppose we could >> also check for non-printing characters, but I don't think ignoring them >> would be a security issue. > > That would be minimum, yes. I guess it would be better/easier to just > limit the names to [a-zA-Z:-_0-9]*? Right, and we also could check if there are no more then two ":" characters in the name. -- Best regards, Jacek Anaszewski