Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752611AbbDQUSY (ORCPT ); Fri, 17 Apr 2015 16:18:24 -0400 Received: from mail-ig0-f174.google.com ([209.85.213.174]:37766 "EHLO mail-ig0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751275AbbDQUSV (ORCPT ); Fri, 17 Apr 2015 16:18:21 -0400 MIME-Version: 1.0 In-Reply-To: <38429B86-0E46-4C6A-9954-3260D864B10C@holtmann.org> References: <300602D8-9DB7-4F8D-AC97-B60924554093@holtmann.org> <38429B86-0E46-4C6A-9954-3260D864B10C@holtmann.org> Date: Fri, 17 Apr 2015 16:18:20 -0400 X-Google-Sender-Auth: yRg2cNwJbGce02f_NPkjzIe5qiA Message-ID: Subject: Re: [V4.1] Regression: Bluetooth mouse not working. From: Linus Torvalds To: Marcel Holtmann Cc: =?UTF-8?Q?J=C3=B6rg_Otte?= , "Gustavo F. Padovan" , Johan Hedberg , "bluez mailin list (linux-bluetooth@vger.kernel.org)" , Linux Kernel Mailing List Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2145 Lines: 52 On Fri, Apr 17, 2015 at 1:02 PM, Marcel Holtmann wrote: > > okay. I only looked at BlueZ 5.x and that might have been my mistake. Let me check this and fix this properly. Why not just revert that commit. It looks like garbage. It has odd code like + u32 valid_flags = 0; + ci->flags = session->flags & valid_flags; which is basically saying "no flags are valid, and we are silently just clearing them all when copying". The reason I think it's garbage is (a) the commit clearly breaks something, so the whole "let's check flags that we've never checked before" is already fundamentally suspicious (b) code like the above is just crap to begin with, because it makes things superficially "look" sensible when looking at individual lines of code (for example, when grepping things), and then when you look at the actual bigger picture, it turns out that the code doesn't actually care about the flags it is "copying", it just clears them all. The other code sequences do things like + u32 valid_flags = 0; + if (req->flags & ~valid_flags) + return -EINVAL; Which again is just a very unreadable way of saying "if any flags are set, return an error". This kind of thing is presumably what breaks things, because clearly people *have* set flags that you thought are invalid. Now *IF* the interfaces had had these kinds of flag validation checks from day one, that would be one thing. But adding these kinds of things after the fact, when somebody then reports that they break things, then that's just a big big flag that you shouldn't try to do this at all. It's water under the bridge. That ship has sailed. It's too late. Give up on it. So I don't think this code is "fixable". It really smells like a fundamental mistake to begin with. Just revert it, chalk it up as "ok, that was a stupid idea", and move on. Linus -- 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/