Bug 218919 - setsockopt() accepts too long arguments allowing programmer errors
Summary: setsockopt() accepts too long arguments allowing programmer errors
Status: New
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Only Me
Assignee: freebsd-net (Nobody)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-04-27 17:39 UTC by Brooks Davis
Modified: 2018-02-24 00:23 UTC (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Brooks Davis freebsd_committer 2017-04-27 17:39:34 UTC
When a programmer mistakenly passes a size_t to a setsockopt() command that takes an int, the sooptcopyin() function ignores the extra size.  On little-endian systems, the result works anyway.  On big-endian systems the wrong bits of the size get copied resulting in failure.  An example of this can be seen in https://reviews.freebsd.org/D10518.

Due to the use of unions as arguments to some setsockopt() commands, it is somewhat clear why sooptcopyin() takes a length and minlength.  It's less clear to me that over-length parameters should be allowed and in the case of int/size_t confusion the current behavior is just wrong.

https://reviews.freebsd.org/D10519 contains one possible partial fix, but I think a larger sweep is required and something more like a sooptcopyin_exact() might be a better approach.
Comment 1 commit-hook freebsd_committer 2017-04-28 17:59:18 UTC
A commit references this bug:

Author: brooks
Date: Fri Apr 28 17:58:16 UTC 2017
New revision: 317566
URL: https://svnweb.freebsd.org/changeset/base/317566

Log:
  Don't pass size_t arguments to setsockopt(SO_SNDBUF/SO_RCVBUF).

  These command take an int. The tests work by accident on little-endian,
  64-bit systems.

  PR:		218919
  Tested with:	qemu-cheri and CheriBSD built for mips64
  Reviewed by:	asomers, ngie
  Obtained from:	CheriBSD
  MFC after:	1 week
  Sponsored by:	DARPA, AFRL
  Differential Revision:	https://reviews.freebsd.org/D10518

Changes:
  head/tests/sys/kern/unix_seqpacket_test.c
Comment 2 commit-hook freebsd_committer 2017-05-05 15:16:15 UTC
A commit references this bug:

Author: brooks
Date: Fri May  5 15:15:42 UTC 2017
New revision: 317831
URL: https://svnweb.freebsd.org/changeset/base/317831

Log:
  MFC r317566:

  Don't pass size_t arguments to setsockopt(SO_SNDBUF/SO_RCVBUF).

  These commands take an int. The tests work by accident on little-endian,
  64-bit systems.

  PR:		218919
  Tested with:	qemu-cheri and CheriBSD built for mips64
  Reviewed by:	asomers, ngie
  Obtained from:	CheriBSD
  Sponsored by:	DARPA, AFRL

Changes:
_U  stable/11/
  stable/11/tests/sys/kern/unix_seqpacket_test.c
Comment 3 commit-hook freebsd_committer 2017-05-05 16:25:12 UTC
A commit references this bug:

Author: brooks
Date: Fri May  5 16:24:35 UTC 2017
New revision: 317834
URL: https://svnweb.freebsd.org/changeset/base/317834

Log:
  MFC r317566:

  Don't pass size_t arguments to setsockopt(SO_SNDBUF/SO_RCVBUF).

  These commands take an int. The tests work by accident on little-endian,
  64-bit systems.

  PR:		218919
  Tested with:	qemu-cheri and CheriBSD built for mips64
  Reviewed by:	asomers, ngie
  Obtained from:	CheriBSD
  Sponsored by:	DARPA, AFRL

Changes:
_U  stable/10/
  stable/10/tests/sys/kern/unix_seqpacket_test.c
Comment 4 Brooks Davis freebsd_committer 2018-02-24 00:23:52 UTC
I did a quick scan of sooptcopyin() use and only a tiny portion of callers call with a minlength less than length.  We should likely separate the two usecases.

Separately, only one case seems to use the support for overly long arguments.  If most cases that support seems harmful since it allows userspace programming errors to be hidden.