Bug 217747 - sys/dev/mly/mly.c: PVS-Studio: NULL Pointer Dereference (CWE-476) (1)
Summary: sys/dev/mly/mly.c: PVS-Studio: NULL Pointer Dereference (CWE-476) (1)
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Many People
Assignee: John Baldwin
URL:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2017-03-13 12:09 UTC by Svyatoslav
Modified: 2017-09-20 23:59 UTC (History)
2 users (show)

See Also:
jhb: mfc-stable11+
jhb: mfc-stable10+

Attachments
git patch (388 bytes, patch)
2017-03-13 12:09 UTC, Svyatoslav 		no flags Details | Diff
mly.patch (794 bytes, patch)
2017-08-08 15:56 UTC, John Baldwin 		no flags Details | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Svyatoslav 2017-03-13 12:09:14 UTC 
Created attachment 180770 [details]
git patch

We have found and fixed a vulnerability using PVS-Studio tool. PVS-Studio is a static code analyzer for C, C++ and C#: https://www.viva64.com/en/pvs-studio/

Analyzer warning: V595 The 'mc' pointer was utilized before it was verified against nullptr. Check lines: 2954, 2955. mly.c 2954
Comment 1 John Baldwin freebsd_committer freebsd_triage 2017-08-08 15:56:02 UTC 
Created attachment 185158 [details]
mly.patch

I would probably either move the check of mc_data into the 'if (mc != NULL)' block, or do the approach above where we just return immediately if we fail to get a command and then 'mc' is never NULL when 'out' is jumped to.
Comment 2 Ed Maste freebsd_committer freebsd_triage 2017-08-08 16:57:57 UTC 
Your patch looks good to me. Will you commit it?
Comment 3 commit-hook freebsd_committer freebsd_triage 2017-08-08 17:50:48 UTC 
A commit references this bug:

Author: jhb
Date: Tue Aug  8 17:49:58 UTC 2017
New revision: 322270
URL: https://svnweb.freebsd.org/changeset/base/322270

Log:
  Fix a NULL pointer dereference in mly_user_command().

  If mly_user_command fails to allocate a command slot it jumps to an 'out'
  label used for error handling.  The error handling code checks for a data
  buffer in 'mc->mc_data' to free before checking if 'mc' is NULL.  Fix by
  just returning directly if we fail to allocate a command and only using
  the 'out' label for subsequent errors when there is actual cleanup to
  perform.

  PR:		217747
  Reported by:	PVS-Studio
  Reviewed by:	emaste
  MFC after:	1 week

Changes:
  head/sys/dev/mly/mly.c
Comment 4 commit-hook freebsd_committer freebsd_triage 2017-09-20 20:49:16 UTC 
A commit references this bug:

Author: jhb
Date: Wed Sep 20 20:48:22 UTC 2017
New revision: 323826
URL: https://svnweb.freebsd.org/changeset/base/323826

Log:
  MFC 322270: Fix a NULL pointer dereference in mly_user_command().

  If mly_user_command fails to allocate a command slot it jumps to an 'out'
  label used for error handling.  The error handling code checks for a data
  buffer in 'mc->mc_data' to free before checking if 'mc' is NULL.  Fix by
  just returning directly if we fail to allocate a command and only using
  the 'out' label for subsequent errors when there is actual cleanup to
  perform.

  PR:		217747
  Reported by:	PVS-Studio

Changes:
_U  stable/10/
  stable/10/sys/dev/mly/mly.c
_U  stable/11/
  stable/11/sys/dev/mly/mly.c