Bug 218919

Summary:	setsockopt() accepts too long arguments allowing programmer errors
Product:	Base System	Reporter:	Brooks Davis <brooks>
Component:	kern	Assignee:	freebsd-net (Nobody) <net>
Status:	New ---
Severity:	Affects Only Me
Priority:	---
Version:	CURRENT
Hardware:	Any
OS:	Any

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.