Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751676AbdFILPo (ORCPT ); Fri, 9 Jun 2017 07:15:44 -0400 Received: from mail-yb0-f179.google.com ([209.85.213.179]:35651 "EHLO mail-yb0-f179.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751646AbdFILPm (ORCPT ); Fri, 9 Jun 2017 07:15:42 -0400 MIME-Version: 1.0 In-Reply-To: <20170608.160425.3981801836671654.davem@davemloft.net> References: <20170608091336.8274-1-mjurczyk@google.com> <20170608.160425.3981801836671654.davem@davemloft.net> From: Mateusz Jurczyk Date: Fri, 9 Jun 2017 13:15:40 +0200 Message-ID: Subject: Re: [PATCH] af_unix: Add sockaddr length checks before accessing sa_family in bind and connect handlers To: David Miller Cc: WANG Cong , Hannes Frederic Sowa , Al Viro , Kees Cook , Miklos Szeredi , Isaac Boukris , Andrey Vagin , netdev@vger.kernel.org, linux-kernel@vger.kernel.org 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: 1685 Lines: 36 On Thu, Jun 8, 2017 at 10:04 PM, David Miller wrote: > From: Mateusz Jurczyk > Date: Thu, 8 Jun 2017 11:13:36 +0200 > >> Verify that the caller-provided sockaddr structure is large enough to >> contain the sa_family field, before accessing it in bind() and connect() >> handlers of the AF_UNIX socket. Since neither syscall enforces a minimum >> size of the corresponding memory region, very short sockaddrs (zero or >> one byte long) result in operating on uninitialized memory while >> referencing .sa_family. >> >> Signed-off-by: Mateusz Jurczyk > > The sockaddr comes from a structure on the caller's kernel stack, even > if the user gives a smaller length, it is legal to access that memory. It is legal to access it, but since it's uninitialized kernel stack memory, the results of comparisons against AF_UNIX or AF_UNSPEC are indeterminate. In practice a user-mode program could likely use timing measurement to infer the evaluation of these comparisons, and hence determine if a garbage 16-bit variable on the kernel stack is equal to 0x0000 or 0x0001, or a garbage byte is equal to 0x00 (if the first byte is provided). This is of course not very bad. However, my project for finding use of uninitialized memory flagged it, and I thought it was worth fixing, at least to avoid having this construct detected in the future (e.g. by KMSAN). There are a few more instances of this behavior in other socket types, which I was going to report with separate patches. If you decide this kind of issues indeed deserves a fix, please let me know if further separate patches are the right approach. Thanks, Mateusz