Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1630766

Re: [PATCH v2] net/packet: initialize val in packet_getsockopt()

From Alexander Potapenko <glider@google.com>
Newsgroups linux.kernel
Subject Re: [PATCH v2] net/packet: initialize val in packet_getsockopt()
Date 2017-04-25 18:40 +0200
Message-ID <tAbFo-77u-29@gated-at.bofh.it> (permalink)
References <tzLKX-75O-27@gated-at.bofh.it> <tAaT1-6BR-49@gated-at.bofh.it> <tAbvI-74c-27@gated-at.bofh.it> <tAbFo-77u-31@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Tue, Apr 25, 2017 at 6:32 PM, David Miller <davem@davemloft.net> wrote:
> From: Alexander Potapenko <glider@google.com>
> Date: Tue, 25 Apr 2017 18:27:04 +0200
>
>> On Tue, Apr 25, 2017 at 5:44 PM, David Miller <davem@davemloft.net> wrote:
>>> From: Alexander Potapenko <glider@google.com>
>>> 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 <glider@google.com>
>>>> ---
>>>> 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

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

Re: [PATCH v2] net/packet: initialize val in packet_getsockopt() David Miller <davem@davemloft.net> - 2017-04-25 17:50 +0200
  Re: [PATCH v2] net/packet: initialize val in packet_getsockopt() Alexander Potapenko <glider@google.com> - 2017-04-25 18:30 +0200
    Re: [PATCH v2] net/packet: initialize val in packet_getsockopt() Alexander Potapenko <glider@google.com> - 2017-04-25 18:40 +0200
    Re: [PATCH v2] net/packet: initialize val in packet_getsockopt() David Miller <davem@davemloft.net> - 2017-04-25 18:40 +0200

csiph-web