Bug 202298

Summary: usr/src/sys/dev/beri/virtio/virtio_block.c:404: 21 into 20 won't go
Product: Base System Reporter: David Binderman <dcb314>
Component: miscAssignee: Oleksandr Tymoshenko <gonzo>
Status: Closed FIXED    
Severity: Affects Some People CC: gonzo
Priority: ---    
Version: CURRENT   
Hardware: Any   
OS: Any   
See Also: https://reviews.freebsd.org/D18852

Description David Binderman 2015-08-13 16:52:30 UTC
[usr/src/sys/dev/beri/virtio/virtio_block.c:404]: (error) Buffer is accessed out of bounds: sc.ident

  sprintf(sc->ident, "Virtio block backend");

but sc->ident is only 20 bytes long and the text string, including
trailing NULL, is 21 bytes long.

Also, since there are no % specifiers in the text string, strcpy
would be faster.
Comment 1 David Binderman 2016-07-27 15:48:34 UTC
*** Bug 211410 has been marked as a duplicate of this bug. ***
Comment 2 David Binderman 2018-07-05 20:00:04 UTC
Still broken three years later.
Comment 3 commit-hook freebsd_committer freebsd_triage 2019-02-11 07:43:19 UTC
A commit references this bug:

Author: gonzo
Date: Mon Feb 11 07:42:32 UTC 2019
New revision: 343998
URL: https://svnweb.freebsd.org/changeset/base/343998

Log:
  Fix off-by-one error in BERI virtio driver

  The hardcoded ident is exactly 20 bytes long but sprintf adds terminating zero,
  so there is one byte written out of array bounds.As a fix use strncpy it
  appends \0 only if space allows and its behavior matches virtio spec:

  When VIRTIO_BLK_T_GET_ID is issued, the device identifier, up to 20 bytes, is
  written to the buffer. The identifier should be interpreted as an ascii string.
  It is terminated with \0, unless it is exactly 20 bytes long.

  PR:		202298
  Reviewed by:	br
  MFC after:	1 week
  Differential Revision:	https://reviews.freebsd.org/D18852

Changes:
  head/sys/dev/beri/virtio/virtio_block.c
Comment 4 commit-hook freebsd_committer freebsd_triage 2019-03-23 23:43:53 UTC
A commit references this bug:

Author: gonzo
Date: Sat Mar 23 23:43:33 UTC 2019
New revision: 345462
URL: https://svnweb.freebsd.org/changeset/base/345462

Log:
  MFC r343998:

  Fix off-by-one error in BERI virtio driver

  The hardcoded ident is exactly 20 bytes long but sprintf adds terminating zero,
  so there is one byte written out of array bounds.As a fix use strncpy it
  appends \0 only if space allows and its behavior matches virtio spec:

  When VIRTIO_BLK_T_GET_ID is issued, the device identifier, up to 20 bytes, is
  written to the buffer. The identifier should be interpreted as an ascii string.
  It is terminated with \0, unless it is exactly 20 bytes long.

  PR:		202298
  Reviewed by:	br
  Differential Revision:	https://reviews.freebsd.org/D18852

Changes:
_U  stable/12/
  stable/12/sys/dev/beri/virtio/virtio_block.c