Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1431997AbdDYQiz (ORCPT ); Tue, 25 Apr 2017 12:38:55 -0400 Received: from mail-qk0-f171.google.com ([209.85.220.171]:34380 "EHLO mail-qk0-f171.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1431814AbdDYQil (ORCPT ); Tue, 25 Apr 2017 12:38:41 -0400 MIME-Version: 1.0 In-Reply-To: <20170425.123201.1910437101082999848.davem@davemloft.net> References: <20170424125914.43270-1-glider@google.com> <20170425.114433.143144279134920277.davem@davemloft.net> <20170425.123201.1910437101082999848.davem@davemloft.net> From: Alexander Potapenko Date: Tue, 25 Apr 2017 18:38:39 +0200 Message-ID: Subject: Re: [PATCH v2] net/packet: initialize val in packet_getsockopt() To: David Miller Cc: Dmitriy Vyukov , Kostya Serebryany , Eric Dumazet , Alexey Kuznetsov , LKML , Networking 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-Transfer-Encoding: 8bit X-MIME-Autoconverted: from quoted-printable to 8bit by mail.home.local id v3PGcwHb025759 Content-Length: 1596 Lines: 42 On Tue, Apr 25, 2017 at 6:32 PM, David Miller wrote: > From: Alexander Potapenko > Date: Tue, 25 Apr 2017 18:27:04 +0200 > >> On Tue, Apr 25, 2017 at 5:44 PM, David Miller wrote: >>> From: Alexander Potapenko >>> Date: Mon, 24 Apr 2017 14:59:14 +0200 >>> >>>> In the case getsockopt() is called with PACKET_HDRLEN and optlen < 4 >>>> |val| remains uninitialized and the syscall may behave differently >>>> depending on its value. This doesn't have security consequences (as the >>>> uninit bytes aren't copied back), but it's still cleaner to initialize >>>> |val| and ensure optlen is not less than sizeof(int). >>>> >>>> This bug has been detected with KMSAN. >>>> >>>> Signed-off-by: Alexander Potapenko >>>> --- >>>> v2: - if len < sizeof(int), make it 0 >>> >>> No, you should signal an error if the len is too small. >> According to manpages, only setsockopt() may return EINVAL. >> Is it ok to change the behavior of getsockopt() to return EINVAL in >> this case? (I.e. won't we break existing users that don't expect it?) > > They are currently getting corrupt data depending upon the endianness, > so -EINVAL is a serious improvement. On a second glance getsockopt() already returns -EINVAL in some cases, so man is already imprecise. -- Alexander Potapenko Software Engineer Google Germany GmbH Erika-Mann-Straße, 33 80636 München Geschäftsführer: Matthew Scott Sucherman, Paul Terence Manicle Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg