# PVS\-Studio is now available on macOS: 64 weaknesses in the Apple's XNU Kernel

A new version of the PVS\-Studio analyzer 6\.23 is working under macOS, which allows you to check the projects written in C and C\+\+\. Our team decided to perform a XNU Kernel check to coincide it with this event\. 

![0566_XNU_Kernel/image1.png](https://import.viva64.com/docx/blog/0566_XNU_Kernel/image1.png)

## PVS\-Studio for macOS

With the release of the analyzer version for macOS, PVS\-Studio can now be boldly called a cross\-platform static code analyzer for C and C\+\+ code\.

Originally, there was only a Windows version\. About two years ago our team supported Linux: "[The Development History of PVS\-Studio for Linux](https://pvs-studio.com/en/blog/posts/cpp/0451/)"\. Also, attentive readers of our blog should remember the articles about the FreeBSD Kernel check \([1st article](https://pvs-studio.com/en/blog/posts/cpp/0377/), [2nd article](https://pvs-studio.com/en/blog/posts/cpp/0496/)\)\. At that time, the analyzer has been built to be launched in PC\-BSD and TrueOS\. Now, finally, we got to macOS\!

**Is it easy to develop a cross\-platform product?**

This issue has economic and technical component\.

From the economic point of view, it was the right decision to make a cross\-platform analyzer\. Software development has long been moving in this direction, and a tool for developers of such projects must be relevant\. However, if something is useful, it does not mean that it is worth doing straight away\. In the beginning, we always make sure we have enough forces to implement something in a new direction, and then maintain it\.

Technically, it is difficult only in the very beginning, if the project is not directly intended as cross\-platform\. We spent a few months on the adaptation of the analyzer in a Linux system\. Compilation of a project under a new platform didn't take much time: we have no GUI and the code is practically not connected with using of the system API\. Analyzer adaptation under new compilers and improvement of analysis quality took most of the time\. In other words, [preventing false positives](https://pvs-studio.com/en/blog/posts/cpp/0488/) requires many efforts\.

**What's with the development under macOS?**

At this point, we already had the analyzer project file for CMake, which was easily adaptable under different operating systems\. Testing systems of different types were also cross\-platform\. All this has helped to start out on macOS\.

The Apple LLVM Compiler became the feature of the analyzer development under macOS\. Although the analyzer was building perfectly using GCC and worked magnificently, but it still could have an impact on analyzer compatibility with users' computers\. To avoid creating problems for potential users, we have decided to support the distribution build using this compiler that comes with Xcode\.

C\+\+ development greatly helps in the creating and development of cross\-platform projects, but different compilers add such capabilities unevenly, so conditional compilation is still actively used in several places\.

In general, everything went smoothly and easily\. As before, most of the time was spent on refinement of the exceptions, site modification, testing and other related issues\. As a first project, checked using PVS\-Studio for macOS we'd like to present you the [XNU Kernel](https://github.com/apple/darwin-xnu)\.

**Distribution package**

Please click [here](https://pvs-studio.com/en/pvs-studio/download/), for further information about the ways to download and install PVS\-Studio for macOS\.

## XNU Kernel

How to start demonstrating the abilities of PVS\-Studio for macOS? No doubts, the check of the kernel of this system is the best variant\! Therefore, the first project, checked using the new version of the analyzer, became the XNU Kernel\.

XNU is a kernel of computer operating systems developed by Apple and used in OS X operating systems \(macOS, iOS, tvOS, watchOS\)\. [Read more](https://en.wikipedia.org/wiki/XNU)\.

It is considered that the kernel is written in C and C\+\+, but in fact, it is C\. I counted 1302 \*\.c\-files and only 97 \*\.cpp\-files\. The size of the codebase is 1929 KLOC\. It turns out that this is a relatively small project\. For comparison, the codebase of the Chromium project is in 15 times larger and contains 30 MLOC\.

The source code can be conveniently downloaded from a mirror on GitHub: [xnu](https://github.com/apple/darwin-xnu)\.

## Results of the check

Although the XNU Kernel is relatively small, it's a challenge to study the analyzer warnings alone, which takes much time\. False positives make the check more complicated, since I haven't performed the preliminary analyzer configuration\.  I just quickly looked through the warnings, writing out code fragments that, in my opinion, represent interest\. This is more than enough for writing a quite large article\. PVS\-Studio analyzer easily finds a large number of interesting bugs\.

**Note for XNU Kernel developers**\. I didn't have an objective to find as many bugs as possible\. Therefore, you should not be guided by the article to fix them\. Firstly, it's awkward, because there is no possibility to navigate along the warnings\. Sure, it is much better to use one of the formats, which can generate PVS\-Studio, for example, the HTML report with the possibility of navigation \(it is similar to something that Clang can generate\)\. Secondly, I skipped many errors simply because I studied the report superficially\. I recommend developers to perform a more thorough analysis of the project with the help of PVS\-Studio themselves\.

As I said, I was bothered with false positives, but in fact, they are no problem\. If you configure the analyzer, it is possible to reduce the number of false positives to [10\-15%](https://pvs-studio.com/en/blog/posts/cpp/0523/)\. As analyzer configuration also requires time and restarting the process of analyzing, I skipped this step \- it wasn't difficult for me to gather errors for the article even without it\. If you plan to perform the analysis carefully, of course, you should take time to make configurations\.

Mostly, false positives occur due to macros and functions marked not qualitatively enough\. For example, in the XNU Kernel, most of them is associated with using of _panic_\.

This is how this function is declared:

```cpp
extern void panic(const char *string, ...)
  __attribute__((__format__ (__printf__, 1, 2)));
```

The function is annotated the way its arguments are interpreted by analogy with the arguments of the _printf_ function\. This enables compilers and analyzers to find errors of incorrect strings formatting\. However, the function is not marked as the one that doesn't return control\.  As a result, the following code produces false positives:

```cpp
if (!ptr)
  panic("zzzzzz");
memcpy(ptr, src, n);
```

Here the analyzer issues the warning that a dereferencing of a null pointer is possible\. From its point of view, after calling the _panic_ function, the _memcpy_ function will be called as well\.

To avoid similar false positives, you must change the annotation of the function by adding _\_\_attribute\_\_\(\(noreturn\)\)_:

```cpp
extern __attribute__((noreturn)) void panic(const char *string, ...)
  __attribute__((__format__ (__printf__, 1, 2)));
```

Now let's see what interesting things I managed to notice in the code of the XNU Kernel\. In total, I noted 64 errors and decided to stop at this beautiful number\. I have grouped the defects according to [Common Weakness Enumeration](https://cwe.mitre.org/), this classification is quite well\-known and it will be easier to understand what errors are a question of this or that chapter\.

### CWE\-570/CWE\-571: Expression is Always False/True

Various errors can lead to [CWE\-570](https://cwe.mitre.org/data/definitions/570.html)/[CWE\-571](https://cwe.mitre.org/data/definitions/571.html), i\.e\. situations where a condition or part of a condition is always false/true\. In the case of the XNU Kernel, all these errors, in my opinion, are related to typos\. PVS\-Studio is generally great at identifying typos\.

**Fragment N1**

```cpp
int
key_parse(
      struct mbuf *m,
      struct socket *so)
{
  ....
  if ((m->m_flags & M_PKTHDR) == 0 ||
      m->m_pkthdr.len != m->m_pkthdr.len) {
    ipseclog((LOG_DEBUG,
              "key_parse: invalid message length.\n"));
    PFKEY_STAT_INCREMENT(pfkeystat.out_invlen);
    error = EINVAL;
    goto senderror;
  }
  ....
}
```

PVS\-Studio warning: V501 CWE\-570 There are identical sub\-expressions 'm\-\>M\_dat\.MH\.MH\_pkthdr\.len' to the left and to the right of the '\!\=' operator\. key\.c 9442

Due to a typo, a class member is compared with itself:

```cpp
m->m_pkthdr.len != m->m_pkthdr.len
```

Part of the condition is always false, and as a result, the length of a message is checked incorrectly\. It turns out that the program will continue handling incorrect data\. Perhaps it's not that scary, but many vulnerabilities are just related to the fact that some input data was unchecked or insufficiently checked\. So this fragment of code is clearly worth paying attention of developers\.

**Fragment N2, N3**

```cpp
#define VM_PURGABLE_STATE_MASK  3

kern_return_t
memory_entry_purgeable_control_internal(...., int *state)
{
  ....
  if ((control == VM_PURGABLE_SET_STATE ||
       control == VM_PURGABLE_SET_STATE_FROM_KERNEL) &&
      (((*state & ~(VM_PURGABLE_ALL_MASKS)) != 0) ||
       ((*state & VM_PURGABLE_STATE_MASK) >
           VM_PURGABLE_STATE_MASK)))
    return(KERN_INVALID_ARGUMENT);
  ....
}
```

PVS\-Studio warning: V560 CWE\-570 A part of conditional expression is always false: \(\(\* state & 3\) \> 3\)\. vm\_user\.c 3415

Let's consider in more detail this part of expression:

```cpp
(*state & VM_PURGABLE_STATE_MASK) > VM_PURGABLE_STATE_MASK
```

If you substitute the value of the macro, you'll get:

```cpp
(*state & 3) > 3
```

Bitwise AND operation may result in only the values 0, 1, 2, or 3\. It is pointless to check whether 0, 1, 2 or 3 is more than 3\. It is very likely that the expression contains a typo\.

As in the previous case, a status is checked incorrectly, which can result in incorrect processing of incorrect \(tainted\) data\.

The same error is detected in the file vm\_map\.c\. Apparently, a part of the code was written using Copy\-Paste\. Warning: V560 CWE\-570 A part of conditional expression is always false: \(\(\* state & 3\) \> 3\)\. vm\_map\.c 15809

**Fragment N4**

```cpp
void
pat_init(void)
{
  boolean_t  istate;
  uint64_t  pat;

  if (!(cpuid_features() & CPUID_FEATURE_PAT))
    return;

  istate = ml_set_interrupts_enabled(FALSE);

  pat = rdmsr64(MSR_IA32_CR_PAT);
  DBG("CPU%d PAT: was 0x%016llx\n", get_cpu_number(), pat);

  /* Change PA6 attribute field to WC if required */
  if ((pat & ~(0x0FULL << 48)) != (0x01ULL << 48)) {
    mtrr_update_action(CACHE_CONTROL_PAT);
  }
  ml_set_interrupts_enabled(istate);
}
```

PVS\-Studio warning: V547 CWE\-571 Expression is always true\. mtrr\.c 692

Let's go through a pointless check, which is likely to have a typo:

```cpp
(pat & ~(0x0FULL << 48)) != (0x01ULL << 48)
```

Let's calculate some expressions:

* \~\(0x0FULL << 48\) \= 0xFFF0FFFFFFFFFFFF
* \(0x01ULL << 48\) \= 0x0001000000000000

The expression _\(pat & \[0xFFF0FFFFFFFFFFFF\]\)_ can not result in the value _0x0001000000000000_\. The condition is always true\. As a result, the function _mtrr\_update\_action_ is always called\.

**Fragment N5**

Here is a typo which, in my opinion, is very beautiful\.

```cpp
typedef enum {
  CMODE_WK = 0,
  CMODE_LZ4 = 1,
  CMODE_HYB = 2,
  VM_COMPRESSOR_DEFAULT_CODEC = 3,
  CMODE_INVALID = 4
} vm_compressor_mode_t;

void vm_compressor_algorithm_init(void) {
  ....
  assertf(((new_codec == VM_COMPRESSOR_DEFAULT_CODEC) ||
           (new_codec == CMODE_WK) ||
           (new_codec == CMODE_LZ4) || (new_codec = CMODE_HYB)),
          "Invalid VM compression codec: %u", new_codec);
  ....
}
```

PVS\-Studio warning: V768 CWE\-571 The expression 'new\_codec \= CMODE\_HYB' is of enum type\. It is odd that it is used as an expression of a Boolean\-type\. vm\_compressor\_algorithms\.c 419

In the process of checking the condition, the variable _new\_codec_ is assigned a value of 2\. As a result, the condition is always true and the assert\-macro actually checks nothing\.

The error could be harmless\. Well, big deal, macro assert didn't check something \- no problem\. However, in addition, the debug version also doesn't work correctly\. The value of the variable _new\_codec_ goes bad and the wrong codec is used, not the one, which was required\. 

**Fragment N6, N7**

```cpp
void
pbuf_copy_back(pbuf_t *pbuf, int off, int len, void *src)
{
  VERIFY(off >= 0);
  VERIFY(len >= 0);
  VERIFY((u_int)(off + len) <= pbuf->pb_packet_len);

  if (pbuf->pb_type == PBUF_TYPE_MBUF)
    m_copyback(pbuf->pb_mbuf, off, len, src);
  else
  if (pbuf->pb_type == PBUF_TYPE_MBUF) {
    if (len)
      memcpy(&((uint8_t *)pbuf->pb_data)[off], src, len);
  } else
    panic("%s: bad pb_type: %d", __func__, pbuf->pb_type);
}
```

PVS\-Studio warning: V517 CWE\-570 The use of 'if \(A\) \{\.\.\.\} else if \(A\) \{\.\.\.\}' pattern was detected\. There is a probability of logical error presence\. Check lines: 340, 343\. pf\_pbuf\.c 340

To clarify, I'll highlight the main point:

```cpp
if (A)
  foo();
else
  if (A)
    Unreachable_code;
  else
    panic();
```

If the _A _condition is true, then the body of the first _if _operator is executed\. If not, a repeated check doesn't make sense and the _panic_ function is called immediately\. A part of the code is generally unattainable\.

Here is an error either in logic, or a typo in one of the conditions\.

Later in this same file, there is the function _pbuf\_copy\_data_, which apparently was written by using Copy\-Paste and contains the same error\. Warning: V517 CWE\-570 The use of 'if \(A\) \{\.\.\.\} else if \(A\) \{\.\.\.\}' pattern was detected\. There is a probability of logical error presence\. Check lines: 358, 361\. pf\_pbuf\.c 358

### CWE\-670: Always\-Incorrect Control Flow Implementation

The defect [CWE\-670](https://cwe.mitre.org/data/definitions/670.html) says that, most likely, in the code something is not working as intended\.

**Fragment N8, N9, N10**

```cpp
static void
in_ifaddr_free(struct ifaddr *ifa)
{
  IFA_LOCK_ASSERT_HELD(ifa);

  if (ifa->ifa_refcnt != 0) {
    panic("%s: ifa %p bad ref cnt", __func__, ifa);
    /* NOTREACHED */
  } if (!(ifa->ifa_debug & IFD_ALLOC)) {
    panic("%s: ifa %p cannot be freed", __func__, ifa);
    /* NOTREACHED */
  }
  if (ifa->ifa_debug & IFD_DEBUG) {
  ....
}
```

PVS\-Studio warning: V646 CWE\-670 Consider inspecting the application's logic\. It's possible that 'else' keyword is missing\. in\.c 2010

Perhaps, there is no error in this code\. However, this place looks very suspiciously:

```cpp
} if (!(ifa->ifa_debug & IFD_ALLOC)) {
```

It's not normal as it's not the done thing\. It would be more logical to start writing _if_ on a new line\. Code authors should check out this place\. Perhaps, the key word _else_ is omitted here and the code should be as follows:

```cpp
} else if (!(ifa->ifa_debug & IFD_ALLOC)) {
```

Or you just need to add a line break, so that this code would confuse neither the analyzer, nor the colleagues maintaining this code\.

Similar suspicious fragments can be found here:

* V646 CWE\-670 Consider inspecting the application's logic\. It's possible that 'else' keyword is missing\. kern\_malloc\.c 836
* V646 CWE\-670 Consider inspecting the application's logic\. It's possible that 'else' keyword is missing\. ipc\_kmsg\.c 4229

**Fragment N11, N12, N13, N14**

```cpp
int
dup2(proc_t p, struct dup2_args *uap, int32_t *retval)
{
  ....
  while ((fdp->fd_ofileflags[new] & UF_RESERVED) == UF_RESERVED)
  {
    fp_drop(p, old, fp, 1);
    procfdtbl_waitfd(p, new);
#if DIAGNOSTIC
    proc_fdlock_assert(p, LCK_MTX_ASSERT_OWNED);
#endif
    goto startover;
  }  
  ....
startover:
  ....
}
```

PVS\-Studio warning: V612 CWE\-670 An unconditional 'goto' within a loop\. kern\_descrip\.c 628

This code is very strange\.  Note that the body of the _while _operator ends with the_ goto_ operator\. In doing so, the operator _'continue'_ is not used the body of the loop\. This means that the body of the loop will be executed no more than once\.

Why create a loop, if it does not perform more than one iteration? Really, it would be better to use the operator _'if'_, then it would not raise any questions\. I think that's an error, and in the cycle something is written wrong\. For example, perhaps, before the operator _'goto'_ there is no condition\.

Similar "one\-time" loops are found 3 more times:

* V612 CWE\-670 An unconditional 'goto' within a loop\. tty\.c 1084
* V612 CWE\-670 An unconditional 'goto' within a loop\. vm\_purgeable\.c 842
* V612 CWE\-670 An unconditional 'return' within a loop\. kern\_credential\.c 930

### Null pointer dereference: CWE\-476, CWE\-628, CWE\-690

There are various reasons because of which null pointer dereferencing may happen and PVS\-Studio analyzer, depending on the situation, can assign them various CWE\-ID:

* [CWE\-476](https://cwe.mitre.org/data/definitions/476.html): NULL Pointer Dereference
* [CWE\-628](https://cwe.mitre.org/data/definitions/628.html): Function Call with Incorrectly Specified Arguments
* [CWE\-690](https://cwe.mitre.org/data/definitions/690.html): Unchecked Return Value to NULL Pointer Dereference 

When writing the article I considered it reasonable to collect all the errors of this type in one section\.

**Fragment N15**

I'll start with complex and large functions\. First, we'll look at the function _netagent\_send\_error\_response_ in which the pointer, passed in the _session_ argument, gets dereferenced\.

```cpp
static int
netagent_send_error_response(
  struct netagent_session *session, u_int8_t message_type,
               u_int32_t message_id, u_int32_t error_code)
{
  int error = 0;
  u_int8_t *response = NULL;
  size_t response_size = sizeof(struct netagent_message_header);
  MALLOC(response, u_int8_t *, response_size,
         M_NETAGENT, M_WAITOK);
  if (response == NULL) {
    return (ENOMEM);
  }
  (void)netagent_buffer_write_message_header(.....);

  if ((error = netagent_send_ctl_data(session->control_unit,
      (u_int8_t *)response, response_size))) {
    NETAGENTLOG0(LOG_ERR, "Failed to send response");
  }

  FREE(response, M_NETAGENT);
  return (error);
}
```

Note that the pointer _session_ is dereferenced in the expression _session\-\>control\_unit_ without any preliminary check\. Whether a dereference of a null pointer occurs or not, depends on what actual arguments will be passed to this function\. 

Now let's see how the function _netagent\_send\_error\_response _discussed above, is used in the function _netagent\_handle\_unregister\_message_\.

```cpp
static void
netagent_handle_unregister_message(
  struct netagent_session *session, ....)
#pragma unused(payload_length, packet, offset)
  u_int32_t response_error = NETAGENT_MESSAGE_ERROR_INTERNAL;

  if (session == NULL) {
    NETAGENTLOG0(LOG_ERR, "Failed to find session");
    response_error = NETAGENT_MESSAGE_ERROR_INTERNAL;
    goto fail;
  }

  netagent_unregister_session_wrapper(session);

  netagent_send_success_response(session, .....);
  return;
fail:
  netagent_send_error_response(
    session, NETAGENT_MESSAGE_TYPE_UNREGISTER, message_id,
    response_error);
}
```

PVS\-Studio warning: V522 CWE\-628 Dereferencing of the null pointer 'session' might take place\. The null pointer is passed into 'netagent\_send\_error\_response' function\. Inspect the first argument\. Check lines: 427, 972\. network\_agent\.c 427

Here Data Flow analysis, implemented in PVS\-Studio, shows itself\. The analyzer notes that if the _session_ pointer was equal to _NULL_, then some information would be written to the log, and then it goes to a label _fails_\.

Next, a call to the function _netagent\_send\_error\_response_ will follow:

```cpp
fail:
  netagent_send_error_response(
    session, NETAGENT_MESSAGE_TYPE_UNREGISTER, message_id,
    response_error);
```

Note that the ill\-fated _session_ pointer that is equal to NULL is passed to the function as an actual argument\.

As we know, in the function _netagent\_send\_error\_response_ there is no protection in this case and a null pointer dereference will occur\.

**Fragment N16**

The next situation is similar to the previous one\. The function code is shorter, but we'll have to deal with it the same slowly and thoroughly\.

```cpp
void *
pf_lazy_makewritable(struct pf_pdesc *pd, pbuf_t *pbuf, int len)
{
  void *p;

  if (pd->lmw < 0)
    return (NULL);

  VERIFY(pbuf == pd->mp);

  p = pbuf->pb_data;
  if (len > pd->lmw) {
  ....
}
```

Note that the pointer _pbuf_ is dereferenced without prior check for _NULL_\. In code there is a check "VERIFY\(pbuf \=\= pd\-\>mp\)"\. However, _pd\-\> mp_ may be equal to _NULL_, so the check cannot be seen as protection against _NULL_\.

Note\. Please, remember that I'm not familiar with the XNU Kernel code and I may be wrong\. Possibly _pd\-\>mp_ will never store the _NULL_ value\. Then all my reasoning is wrong and there is no error here\. However, such code still needs to be checked again\.

Let's continue and see how to the described function _pf\_lazy\_makewritable_ is used\.

```cpp
static int
pf_test_state_icmp(....)
{
  ....
  if (pf_lazy_makewritable(pd, NULL,
      off + sizeof (struct icmp6_hdr)) ==
      NULL)
    return (PF_DROP);
  ....
}
```

PVS\-Studio warning: V522 CWE\-628 Dereferencing of the null pointer 'pbuf' might take place\. The null pointer is passed into 'pf\_lazy\_makewritable' function\. Inspect the second argument\. Check lines: 349, 7460\. pf\.c 349

_NULL_ is passed to the function _pf\_lazy\_makewritable_ as the second actual argument\. This is very strange\.

Let's say, a programmer thinks that "VERIFY\(pbuf \=\= pd\-\>mp\)" will protect the program from the null pointer\. Then the question arises: why write such code? Why call a function passing clearly incorrect argument?

Therefore, it seems to me that actually, the function _pf\_lazy\_makewritable_ must be able to accept a null pointer and handle this case in a special way, but it doesn't do so\. This code deserves thorough verification by a programmer, and the PVS\-Studio analyzer is definitely right, drawing our attention to it\.

**Fragment N17**

Let's relax for a while and consider a simple case\.

```cpp
typedef struct vnode * vnode_t;

int 
cache_lookup_path(...., vnode_t dp, ....)
{
  ....
  if (dp && (dp->v_flag & VISHARDLINK)) {
    break;
  }
  if ((dp->v_flag & VROOT)  ||
      dp == ndp->ni_rootdir ||
      dp->v_parent == NULLVP)
    break;
  ....
}
```

PVS\-Studio warning: V522 CWE\-690 There might be dereferencing of a potential null pointer 'dp'\. vfs\_cache\.c 1449

Look at the check:

```cpp
if (dp && (dp->v_flag & VISHARDLINK))
```

It tells us that a pointer _dp_ can be null\. However further on, the pointer is dereferenced before the preliminary check:

```cpp
if ((dp->v_flag & VROOT) || ....)
```

**Fragment N18**

In the previous example, we saw a situation where the pointer was checked before dereference, and then check in code was forgotten\. But much more often you may come across a situation when pointer is dereferenced first, and only then is checked\. The code of the XNU Kernel project was no exception\. First, let's consider a synthetic sample for better understanding what it is about:

```cpp
p[n] = 1;
if (!p) return false;
```

Now let's see how these errors look like in reality\. We'll start with the function of names comparison\. The comparison functions [are very insidious](https://pvs-studio.com/en/blog/posts/cpp/0509/) :\)\.

```cpp
bool
IORegistryEntry::compareName(....) const
{
  const OSSymbol *  sym = copyName();
  bool    isEqual;

  isEqual = sym->isEqualTo( name );   // <=

  if( isEqual && matched) {
    name->retain();
    *matched = name;
  }

  if( sym)                            // <=
    sym->release();
  return( isEqual );
}
```

PVS\-Studio warnings: V595 CWE\-476 The 'sym' pointer was utilized before it was verified against nullptr\. Check lines: 889, 896\. IORegistryEntry\.cpp 889

I've marked with comments like "//< \=" lines of code which are of interest for us\. As you can see, the first pointer is dereferenced\. Further, in code, there is a check for pointer equality to _nullptr_\. But it is clear at once that if the pointer is null, then there will be a null pointer dereferencing and function, in fact, is not ready for such a situation\.

**Fragment N19**

The following error occurred because of a typo\.

```cpp
static int
memorystatus_get_priority_list(
  memorystatus_priority_entry_t **list_ptr, size_t *buffer_size,
  size_t *list_size, boolean_t size_only) 
{
  ....
  *list_ptr = (memorystatus_priority_entry_t*)kalloc(*list_size);
  if (!list_ptr) {
    return ENOMEM;
  }
  ....
}
```

PVS\-Studio warning: V595 CWE\-476 The 'list\_ptr' pointer was utilized before it was verified against nullptr\. Check lines: 7175, 7176\. kern\_memorystatus\.c 7175

The analyzer sees that the variable is first dereferenced, and in the following line is checked for equality to _nullptr_\. This interesting error occurred due to the fact that the programmer forgot to write the character '\*'\. Actually, correct code should be as follows:

```cpp
*list_ptr = (memorystatus_priority_entry_t*)kalloc(*list_size);
if (!*list_ptr) {
  return ENOMEM;
}
```

We can say that the error was identified indirectly\. However, it does not matter, because the most important thing is that the analyzer drew our attention to abnormal code and we saw the error\.

**Fragment N20 \- N35**

In the XNU Kernel code there are many errors identified thanks to the V595 diagnostic\. However, considering all of them will be boring\. So, I will regard just one case, and cite a list of messages that indicate errors\.

```cpp
inline void
inp_decr_sndbytes_unsent(struct socket *so, int32_t len)
{
  struct inpcb *inp = (struct inpcb *)so->so_pcb;
  struct ifnet *ifp = inp->inp_last_outifp;

  if (so == NULL || !(so->so_snd.sb_flags & SB_SNDBYTE_CNT))
    return;

  if (ifp != NULL) {
    if (ifp->if_sndbyte_unsent >= len)
      OSAddAtomic64(-len, &ifp->if_sndbyte_unsent);
    else
      ifp->if_sndbyte_unsent = 0;
  }
}
```

PVS\-Studio warning: V595 CWE\-476 The 'so' pointer was utilized before it was verified against nullptr\. Check lines: 3450, 3453\. in\_pcb\.c 3450

I suggest the reader to independently follow the fate of the pointer _so_ and make sure, that the code is written incorrectly\.

Other errors:

* V595 CWE\-476 The 'startDict' pointer was utilized before it was verified against nullptr\. Check lines: 3369, 3373\. IOService\.cpp 3369
* V595 CWE\-476 The 'job' pointer was utilized before it was verified against nullptr\. Check lines: 4083, 4085\. IOService\.cpp 4083
* V595 CWE\-476 The 'typeinst' pointer was utilized before it was verified against nullptr\. Check lines: 176, 177\. OSMetaClass\.cpp 176
* V595 CWE\-476 The 'name' pointer was utilized before it was verified against nullptr\. Check lines: 385, 392\. devfs\_tree\.c 385
* V595 CWE\-476 The 'collection' pointer was utilized before it was verified against nullptr\. Check lines: 71, 75\. OSCollectionIterator\.cpp 71
* V595 CWE\-476 The 'ifp' pointer was utilized before it was verified against nullptr\. Check lines: 2014, 2018\. dlil\.c 2014
* V595 CWE\-476 The 'fakeif' pointer was utilized before it was verified against nullptr\. Check lines: 561, 566\. if\_fake\.c 561
* V595 CWE\-476 The 'sb' pointer was utilized before it was verified against nullptr\. Check lines: 138, 140\. in\_pcblist\.c 138
* V595 CWE\-476 The 'tp' pointer was utilized before it was verified against nullptr\. Check lines: 2603, 2610\. tcp\_subr\.c 2603
* V595 CWE\-476 The 'str\_id' pointer was utilized before it was verified against nullptr\. Check lines: 1812, 1817\. kdebug\.c 1812
* V595 CWE\-476 The 'sessp' pointer was utilized before it was verified against nullptr\. Check lines: 191, 194\. subr\_prf\.c 191
* V595 CWE\-476 The 'sessp' pointer was utilized before it was verified against nullptr\. Check lines: 1463, 1469\. tty\.c 1463
* V595 CWE\-476 The 'so' pointer was utilized before it was verified against nullptr\. Check lines: 6714, 6719\. uipc\_socket\.c 6714
* V595 CWE\-476 The 'uap' pointer was utilized before it was verified against nullptr\. Check lines: 314, 320\. nfs\_upcall\.c 314
* V595 CWE\-476 The 'xfromname' pointer was utilized before it was verified against nullptr\. Check lines: 3986, 4006\. kpi\_vfs\.c 3986
* Note\. Actually I didn't look carefully through all the warnings of this type\. Therefore, actually there may be more errors\.

**Fragment N36, N37**

And the last couple bugs on the use of NULL pointers\.

```cpp
static void
feth_start(ifnet_t ifp)
{
  ....
  if_fake_ref  fakeif;
  ....
  if (fakeif != NULL) {
    peer = fakeif->iff_peer;
    flags = fakeif->iff_flags;
  }

  /* check for pending TX */
  m = fakeif->iff_pending_tx_packet;
  ....
}
```

PVS\-Studio warning: V1004 CWE\-476 The 'fakeif' pointer was used unsafely after it was verified against nullptr\. Check lines: 566, 572\. if\_fake\.c 572

I think, this code doesn't need any comments\. Just look how the pointer _fakeif_ is checked and used\.

The last similar case: V1004 CWE\-476 The 'rt\-\>rt\_ifp' pointer was used unsafely after it was verified against nullptr\. Check lines: 138, 140\. netsrc\.c 140

### CWE\-119: Improper Restriction of Operations within the Bounds of a Memory Buffer

I came across a couple of errors, related to the buffer overrun\.  A very unpleasant kind of error for such a responsible project, like XNU Kernel\.

Different variants of array overrun can be classified with different CWE ID, but in this case, the analyzer chose [CWE\-119](https://cwe.mitre.org/data/definitions/119.html)\.

**Fragment N38**

For a start, let's see how some macros are declared\. 

```cpp
#define  IFNAMSIZ   16
#define  IFXNAMSIZ  (IFNAMSIZ + 8)
#define MAX_ROUTE_RULE_INTERFACES 10
```

It is important for us to remember that:

* IFXNAMSIZ \= 24
* MAX\_ROUTE\_RULE\_INTERFACES \= 10

And now we'll look at the function where the buffer overrun is possible when using the _snprintf_ and _memset_ functions\. So, 2 errors take place here\. 

```cpp
static inline const char *
necp_get_result_description(....)
{
  ....
  char interface_names[IFXNAMSIZ][MAX_ROUTE_RULE_INTERFACES];
  ....
  for (index = 0; index < MAX_ROUTE_RULE_INTERFACES; index++) {
    if (route_rule->exception_if_indices[index] != 0) {
      ifnet_t interface = ifindex2ifnet[....];
      snprintf(interface_names[index],
               IFXNAMSIZ, "%s%d", ifnet_name(interface),
               ifnet_unit(interface));
    } else {
      memset(interface_names[index], 0, IFXNAMSIZ);
    }
  }
  ....
}
```

PVS\-Studio warnings:

* V512 CWE\-119 A call of the '\_\_builtin\_\_\_memcpy\_chk' function will lead to a buffer overflow\. \- ADDITIONAL IN CURRENT necp\_client\.c 1459
* V557 CWE\-787 Array overrun is possible\. The value of 'length \- 1' index could reach 23\. \- ADDITIONAL IN CURRENT necp\_client\.c 1460

Notice how the two\-dimensional array _interface\_names_ is declared:

```cpp
char interface_names[IFXNAMSIZ][MAX_ROUTE_RULE_INTERFACES];
// i.g.: char interface_names[24][10];
```

But this array is used as if it is as follows:

```cpp
char interface_names[MAX_ROUTE_RULE_INTERFACES][IFXNAMSIZ];
// i.g.: char interface_names[10][24];
```

In the result we get a mush of data\. 

Someone may say without thinking, that there is nothing to worry about, because both arrays hold the same number of bytes\.

No, it's bad\. The elements of the array _interface\_names\[10\.\.23\]\[\.\.\.\.\]_ are not used, because the variable _index_ in the loop takes values \[0\.\.9\]\. But the elements of _interface\_names\[0\.\.9\]\[\.\.\.\.\]_ begin to overlap each other\. I\.e\. some data overwrites the other\.

The result is just nonsense\. A part of the array remains uninitialized, and the other part contains a "mush", when data was written over the already written data\.

**Fragment N39**

Later in this same file _necp\_client\.c_ there is another function that contains very similar errors\.

```cpp
#define  IFNAMSIZ   16
#define  IFXNAMSIZ  (IFNAMSIZ + 8)

#define NECP_MAX_PARSED_PARAMETERS 16

struct necp_client_parsed_parameters {
  ....
  char prohibited_interfaces[IFXNAMSIZ]
                                  [NECP_MAX_PARSED_PARAMETERS];
  ....
};

static int
necp_client_parse_parameters(....,
  struct necp_client_parsed_parameters *parsed_parameters)
{
  ....
  u_int32_t length = ....;
  ....
  if (length <= IFXNAMSIZ && length > 0) {
    memcpy(parsed_parameters->prohibited_interfaces[
                                     num_prohibited_interfaces],
           value, length);
    parsed_parameters->prohibited_interfaces[
                    num_prohibited_interfaces][length - 1] = 0;
  ....
}
```

PVS\-Studio warning:

* V512 CWE\-119 A call of the '\_\_builtin\_\_\_memcpy\_chk' function will lead to a buffer overflow\. \- ADDITIONAL IN CURRENT necp\_client\.c 1459
* V557 CWE\-787 Array overrun is possible\. The value of 'length \- 1' index could reach 23\. \- ADDITIONAL IN CURRENT necp\_client\.c 1460

All the same\. The array:

```cpp
char prohibited_interfaces[IFXNAMSIZ][NECP_MAX_PARSED_PARAMETERS];
```

is handled as if it is:

```cpp
char prohibited_interfaces[NECP_MAX_PARSED_PARAMETERS][IFXNAMSIZ];
```

### CWE\-563: Assignment to Variable without Use

Defects [CWE\-563](https://cwe.mitre.org/data/definitions/563.html) detected by PVS\-Studio are often the consequences of typos\. Now we'll consider one such beautiful typo\.

**Fragment N40**

```cpp
uint32_t
gss_krb5_3des_unwrap_mbuf(....)
{
  ....
  for (cflag = 1; cflag >= 0; cflag--) {
    *minor = gss_krb5_3des_token_get(
       ctx, &itoken, wrap, &hash, &offset, &length, reverse);
    if (*minor == 0)
      break;
    wrap.Seal_Alg[0] = 0xff;
    wrap.Seal_Alg[0] = 0xff;
  }
  ....
}
```

PVS\-Studio warning: V519 CWE\-563 The 'wrap\.Seal\_Alg\[0\]' variable is assigned values twice successively\. Perhaps this is a mistake\. Check lines: 2070, 2071\. gss\_krb5\_mech\.c 2071

The value _0xff_ is written in the same element of the array twice\. I looked at the code and concluded that the programmers actually wanted to write here:

```cpp
wrap.Seal_Alg[0] = 0xff;
wrap.Seal_Alg[1] = 0xff;
```

Judging by the name of the function, it is associated with a network authentication protocol\. And such a kludge\. Just terrifying\. 

You can buy PVS\-Studio [here](https://pvs-studio.com/en/order/)\. Our analyzer will help prevent many of these errors\!

**Fragment N41, N42, N43, N44**

```cpp
static struct mbuf *
pf_reassemble(struct mbuf *m0, struct pf_fragment **frag,
    struct pf_frent *frent, int mff)
{
  ....
  m->m_pkthdr.csum_flags &= ~CSUM_PARTIAL;
  m->m_pkthdr.csum_flags =
      CSUM_DATA_VALID | CSUM_PSEUDO_HDR |
      CSUM_IP_CHECKED | CSUM_IP_VALID;
  ....
}
```

PVS\-Studio warning: V519 CWE\-563 The 'm\-\>M\_dat\.MH\.MH\_pkthdr\.csum\_flags' variable is assigned values twice successively\. Perhaps this is a mistake\. Check lines: 758, 759\. pf\_norm\.c 759

String:

```cpp
m->m_pkthdr.csum_flags &= ~CSUM_PARTIAL;
```

has no practical meaning\. In the next string the variable _m\-\>m\_pkthdr\.csum\_flags_ will be assigned a new value\. I don't know how the correct code should actually look like, but I would venture to guess that the symbol '\|' was lost\. In my humble opinion, your code should look like this:

```cpp
m->m_pkthdr.csum_flags &= ~CSUM_PARTIAL;
m->m_pkthdr.csum_flags |=
    CSUM_DATA_VALID | CSUM_PSEUDO_HDR |
    CSUM_IP_CHECKED | CSUM_IP_VALID;
```

There are 3 warnings pointing at similar errors:

* V519 CWE\-563 The 'm\-\>M\_dat\.MH\.MH\_pkthdr\.csum\_flags' variable is assigned values twice successively\. Perhaps this is a mistake\. Check lines: 1349, 1350\. pf\_norm\.c 1350
* V519 CWE\-563 The 'm\-\>M\_dat\.MH\.MH\_pkthdr\.csum\_flags' variable is assigned values twice successively\. Perhaps this is a mistake\. Check lines: 2984, 2985\. ip\_input\.c 2985
* V519 CWE\-563 The 'm\-\>M\_dat\.MH\.MH\_pkthdr\.csum\_flags' variable is assigned values twice successively\. Perhaps this is a mistake\. Check lines: 773, 774\. frag6\.c 774

### CWE\-14: Compiler Removal of Code to Clear Buffers

A very insidious type of defect that is invisible in the debug version\. If the reader is not familiar with it yet, before you continue reading, I suggest to be acquainted with the following links:

1. [Safe Clearing of Private Data](https://pvs-studio.com/en/blog/posts/cpp/0388/)\.
1. [V597](https://pvs-studio.com/en/docs/warnings/v597/)\. The compiler could delete the 'memset' function call, which is used to flush 'Foo' buffer\. The RtlSecureZeroMemory\(\) function should be used to erase the private data\.
1. [CWE\-14](https://cwe.mitre.org/data/definitions/14.html): Compiler Removal of Code to Clear Buffers\.

If the reader wonders why overwrite private data that is stored in the memory, I recommend the article "[Overwriting memory \- why?](https://pvs-studio.com/en/blog/posts/cpp/k0041/)"\.

So, it is important to overwrite private data in memory, but sometimes the compiler removes the corresponding code, because, from its point of view, it is redundant\. Let's see what interesting things were found in the XNU Kernel on this topic\.

**Fragment N45**

```cpp
__private_extern__ void
YSHA1Final(unsigned char digest[20], YSHA1_CTX* context)
{
  u_int32_t i, j;
  unsigned char finalcount[8];

  ....
  /* Wipe variables */
  i = j = 0;
  memset(context->buffer, 0, 64);
  memset(context->state, 0, 20);
  memset(context->count, 0, 8);
  memset(finalcount, 0, 8);           // <=
#ifdef SHA1HANDSOFF
  YSHA1Transform(context->state, context->buffer);
#endif
}
```

PVS\-Studio warning: V597 CWE\-14 The compiler could delete the 'memset' function call, which is used to flush 'finalcount' buffer\. The memset\_s\(\) function should be used to erase the private data\. sha1mod\.c 188

The compiler may remove the line of code which I marked with the comment "// <\=" in order to optimize the Release\-version\.  Almost certainly, it will act in this way\.

**Fragment N46**

```cpp
__private_extern__ void
YSHA1Transform(u_int32_t state[5], const unsigned char buffer[64])
{
  u_int32_t a, b, c, d, e;
  ....
  state[0] += a;
  state[1] += b;
  state[2] += c;
  state[3] += d;
  state[4] += e;
  /* Wipe variables */
  a = b = c = d = e = 0;
}
```

PVS\-Studio warning: V1001 CWE\-563 The 'a' variable is assigned but is not used until the end of the function\. sha1mod\.c 120

The compiler may not generate code that resets the variables, since they are not used in the function\.

I would like to draw your attention to the fact that PVS\-Studio analyzer interpreted this suspicious situation as [CWE\-563](https://cwe.mitre.org/data/definitions/563.html)\. The fact of the matter is that the same defect can often be interpreted as different CWE and in this case, the analyzer chose CWE\-563\. However, I decided to include this code to CWE\-14 because it explains more accurately, what's wrong with this code\.

### CWE\-783: Operator Precedence Logic Error

The defect [CWE\-783](https://cwe.mitre.org/data/definitions/783.html) occurs where the programmer confused priorities of the operations and wrote code that works not the way he had planned\. Often these errors are made because of carelessness or missing parentheses\.

**Fragment N47**

```cpp
int
getxattr(....)
{
  ....
  if ((error = copyinstr(uap->attrname, attrname,
                         sizeof(attrname), &namelen) != 0)) {
    goto out;
  }
  ....
out:
  ....
  return (error);
}
```

PVS\-Studio warning: V593 CWE\-783 Consider reviewing the expression of the 'A \= B \!\= C' kind\. The expression is calculated as following: 'A \= \(B \!\= C\)'\. vfs\_syscalls\.c 10574

A classic error\. I meet a lot of such bugs in various programs \([proof](https://pvs-studio.com/en/blog/examples/v593/)\)\. The root cause is that for some reason programmers seek to cram more just in one line\.

As a result, instead of:

```cpp
Status s = foo();
if (s == Error)
  return s;
```

they write:

```cpp
Status s;
if (s = foo() == Error)
  return s;
```

And contribute the error to the code\.

* The programmer expects that the expression is evaluated as follows: \(s \= foo\(\)\) \=\= Error\.
* Actually, the expression is evaluated as follows: s \= \(foo\(\) \=\= Error\)\.

As a result, the _return _operator returns incorrect error status equal to 1, but not the value that is equal to a constant _Error_\.

I regularly criticize such code and recommend not to "shove in" in one line more than one action\. "Stuffing in" doesn't really reduce the code size, but provokes a different error\. See the chapter 13 from the book "[The Ultimate Question of Programming, Refactoring, and Everything](https://pvs-studio.com/en/blog/posts/cpp/0391/)" for more details\.  See the chapters:

* 11\. Don't be greedy on the lines of code
* 16\. "Look what I can do\!" \- unacceptable in programming

Let's get back to code from the XNU Kernel\. In case of an error, the function _getxattr_ will return value of 1, not the actual error code\.

**Fragment N48\-N52**

```cpp
static void
memorystatus_init_snapshot_vmstats(
  memorystatus_jetsam_snapshot_t *snapshot)
{
  kern_return_t kr = KERN_SUCCESS;
  mach_msg_type_number_t  count = HOST_VM_INFO64_COUNT;
  vm_statistics64_data_t  vm_stat;

  if ((kr = host_statistics64(.....) != KERN_SUCCESS)) {
    printf("memorystatus_init_jetsam_snapshot_stats: "
           "host_statistics64 failed with %d\n", kr);
    memset(&snapshot->stats, 0, sizeof(snapshot->stats));
  } else {
+  ....
}
```

PVS\-Studio warning: V593 CWE\-783 Consider reviewing the expression of the 'A \= B \!\= C' kind\. The expression is calculated as following: 'A \= \(B \!\= C\)'\. kern\_memorystatus\.c 4554

Variable _kr_ can be assigned only two values: 0 or 1\. Due to this _printf _function always prints the number 1 instead of the actual status, which the function _host\_statistics64_ returned\.

Article turns out to be large\.  I guess I'm tiring not only myself, but also the readers\. So I'm reducing the number of fragments regarded in the article\.

Other similar defects are uninteresting to be considered, and I shall confine myself to the message list:

* V593 CWE\-783 Consider reviewing the expression of the 'A \= B \!\= C' kind\. The expression is calculated as following: 'A \= \(B \!\= C\)'\. vfs\_syscalls\.c 10654
* V593 CWE\-783 Consider reviewing the expression of the 'A \= B \!\= C' kind\. The expression is calculated as following: 'A \= \(B \!\= C\)'\. vfs\_syscalls\.c 10700
* V593 CWE\-783 Consider reviewing the expression of the 'A \= B \!\= C' kind\. The expression is calculated as following: 'A \= \(B \!\= C\)'\. vfs\_syscalls\.c 10759
* V593 CWE\-783 Consider reviewing the expression of the 'A \= B \!\= C' kind\. The expression is calculated as following: 'A \= \(B \!\= C\)'\. kern\_exec\.c 2297

### CWE\-758: Reliance on Undefined, Unspecified, or Implementation\-Defined Behavior

There is an enormous number of ways how to get undefined or unspecified behavior in program written in C or C\+\+\. Therefore, PVS\-Studio provides quite a lot of diagnostics aimed at identifying such problems: V567, V610, V611, V681, V704, V708, V726, V736\.

In the case of XNU, the analyzer has identified only two weaknesses [CWE\-758](https://cwe.mitre.org/data/definitions/758.html), related to undefined behavior caused by a shift of negative numbers\.

**Fragment N53, N54**

```cpp
static void
pfr_prepare_network(union sockaddr_union *sa, int af, int net)
{
  ....
  sa->sin.sin_addr.s_addr = net ? htonl(-1 << (32-net)) : 0;
  ....
}
```

PVS\-Studio warning: V610 CWE\-758 Undefined behavior\. Check the shift operator '<<'\. The left operand '\-1' is negative\. pf\_table\.c 976

Shift of a negative number to the left leads to undefined behavior\. In practice, this code may work well exactly as the programmer expects\. But still, this code is incorrect and should be corrected\. This can be done in the following way:

```cpp
htonl((unsigned)(-1) << (32-net))
```

PVS\-Studio analyzer finds another shift here: V610 CWE\-758 Undefined behavior\. Check the shift operator '<<'\. The left operand '\-1' is negative\. pf\_table\.c 983

### CWE\-401: Improper Release of Memory Before Removing Last Reference \('Memory Leak'\)

XNU Kernel developers should be praised for the fact that the analyzer could not find any problems with memory leaks \([CWE\-401](https://cwe.mitre.org/data/definitions/401.html)\)\. There are only 3 suspicious places when the _delete_ operator is not called when the object initialization error\. While I'm not sure that this is an error\.

**Fragment N55, N56, N57**

```cpp
IOService * IODTPlatformExpert::createNub(IORegistryEntry * from)
{
  IOService *    nub;

  nub = new IOPlatformDevice;
  if (nub) {
    if( !nub->init( from, gIODTPlane )) {
      nub->free();
      nub = 0;
    }
  }
  return (nub);
}
```

V773 CWE\-401 The 'nub' pointer was assigned values twice without releasing the memory\. A memory leak is possible\. IOPlatformExpert\.cpp 1287

If the function _init _is not able to initialize an object, possibly a memory leak will occur\. In my opinion, it lacks the operator _delete,_ and should have been written like this:

```cpp
if( !nub->init( from, gIODTPlane )) {
  nub->free();
  delete nub;
  nub = 0;
}
```

I'm not sure that I'm right\. Perhaps, the function _free_ destroys the object itself, performing the operation "delete \*this;"\. I didn't carefully sort all that out, because by the time I reached those warnings I was already tired\.

Similar analyzer warnings: 

* V773 CWE\-401 The 'inst' pointer was assigned values twice without releasing the memory\. A memory leak is possible\. IOUserClient\.cpp 246
* V773 CWE\-401 The 'myself' pointer was assigned values twice without releasing the memory\. A memory leak is possible\. IOPMrootDomain\.cpp 9151

### CWE\-129: Improper Validation of Array Index

The defect [CWE\-129](https://cwe.mitre.org/data/definitions/129.html) says that the variables, used for indexing of elements in the array, are incorrectly or insufficiently verified\. Consequently, the array overrun may occur\.

**Fragment N58\-N61**

```cpp
IOReturn
IOStateReporter::updateChannelValues(int channel_index)
{
  ....
  state_index = _currentStates[channel_index];
    
  if (channel_index < 0 ||
      channel_index > (_nElements - state_index)
                        / _channelDimension) {
    result = kIOReturnOverrun; goto finish;
  }
  ....
}
```

PVS\-Studio warning: V781 CWE\-129 The value of the 'channel\_index' variable is checked after it was used\. Perhaps there is a mistake in program logic\. Check lines: 852, 855\. IOStateReporter\.cpp 852

Negative values protection is implemented improperly\. First, the element is retrieved from an array, and only after that, the check follows that the index isn't negative\. 

I think this code should be rewritten as follows:

```cpp
IOReturn
IOStateReporter::updateChannelValues(int channel_index)
{
  ....
  if (channel_index < 0)
  {
    result = kIOReturnOverrun; goto finish;
  }

  state_index = _currentStates[channel_index];
    
  if (channel_index > (_nElements - state_index)
                        / _channelDimension) {
    result = kIOReturnOverrun; goto finish;
  }
  ....
}
```

You may need to add checks that the value _channel\_index_ is not greater than the size of the array\. I'm not familiar with the code, so I'll leave it to the discretion of the XNU Kernel developers\.

Similar errors:

* V781 CWE\-129 The value of the 'channel\_index' variable is checked after it was used\. Perhaps there is a mistake in program logic\. Check lines: 651, 654\. IOStateReporter\.cpp 651
* V781 CWE\-129 The value of the 'pri' variable is checked after it was used\. Perhaps there is a mistake in program logic\. Check lines: 267, 269\. pktsched\_fq\_codel\.c 267
* V781 CWE\-129 The value of the 'pcid' variable is checked after it was used\. Perhaps there is a mistake in program logic\. Check lines: 224, 225\. pmap\_pcid\.c 224

### CWE\-480: Use of Incorrect Operator

CWE\-480 defects are commonly related to some typos in expressions\. There are usually not very much of them, but they are very fun\. You just look at the errors and wonder how they could be done\. However, as we have already demonstrated in the articles that no one is insured from such errors, even highly skilled programmers\.

**Fragment N62**

```cpp
#define NFS_UC_QUEUE_SLEEPING  0x0001
static void
nfsrv_uc_proxy(socket_t so, void *arg, int waitflag)
{
  ....
  if (myqueue->ucq_flags | NFS_UC_QUEUE_SLEEPING)
    wakeup(myqueue);
  ....
}
```

PVS\-Studio warning: V617 CWE\-480 Consider inspecting the condition\. The '0x0001' argument of the '\|' bitwise operation contains a non\-zero value\. nfs\_upcall\.c 331

An essence "awakes" more often that it's needed\.  Rather, it "is woken" constantly, regardless of the conditions\. Most likely, the code here is supposed to be as follows:

```cpp
if (myqueue->ucq_flags & NFS_UC_QUEUE_SLEEPING)
  wakeup(myqueue);
```

### CWE\-665: Improper Initialization

PVS\-Studio analyzer was unable to classify the following error according to CWE\. From my point of view, we are dealing with [CWE\-665](https://cwe.mitre.org/data/definitions/665.html)\.

**Fragment N63**

```cpp
extern void bzero(void *, size_t);

static struct thread  thread_template, init_thread;

struct thread {
  ....
  struct thread_qos_override {
    struct thread_qos_override  *override_next;
    uint32_t  override_contended_resource_count;
    int16_t    override_qos;
    int16_t    override_resource_type;
    user_addr_t  override_resource;
  } *overrides;
  ....
};

void
thread_bootstrap(void)
{
  ....
  bzero(&thread_template.overrides,
        sizeof(thread_template.overrides));
  ....
}
```

PVS\-Studio warning: V568 It's odd that 'sizeof\(\)' operator evaluates the size of a pointer to a class, but not the size of the 'thread\_template\.overrides' class object\. thread\.c 377

A programmer took the address of the variable, containing a pointer and nullified the variable, using the _bzero _function\. In fact, just recorded _nullptr_ in the pointer\.

To use the _bzero_ function is a very strange unnatural way to reset the value of the variable\. It would be much easier to write:

```cpp
thread_template.overrides = NULL;
```

Hence, I conclude that a programmer wanted to reset the buffer, but occasionally nullified the pointer\. Therefore, correct code should be like this:

```cpp
bzero(thread_template.overrides,
      sizeof(*thread_template.overrides));
```

### CWE\-691: Insufficient Control Flow Management

[CWE\-691](https://cwe.mitre.org/data/definitions/691.html) reveals anomalies in the sequence of  instructions execution\. Another anomaly is also possible \- the code presentation doesn't correspond to the way it works\.  I faced exactly this case in the XNU Kernel code\. 

**Fragment N64**

Hooray, we got to the last code fragment\! There may be other errors that I didn't notice when viewing the report, issued by the analyzer, but I'd like to remind that it was not my purpose to identify as many errors as possible\. In any case, developers of the XNU Kernel will be able to study the report better, because they are familiar with the project code\. So let's stop at the beautiful number 64 that is consonant with the name of our site [viva64](https://pvs-studio.com/en/)\.

Note\. For those who wonder where "viva64" came from, I suggest to get acquainted with the section "[PVS\-Studio project \- 10 years of failures and successes](https://pvs-studio.com/en/blog/posts/0465/)\.

```cpp
void vm_page_release_startup(vm_page_t mem);
void
pmap_startup(
  vm_offset_t *startp,
  vm_offset_t *endp)
{
  ....
  // -debug code remove
  if (2 == vm_himemory_mode) {
    for (i = 1; i <= pages_initialized; i++) {
      ....
    }
  }
  else
  // debug code remove-

  /*
   * Release pages in reverse order so that physical pages
   * initially get allocated in ascending addresses. This keeps
   * the devices (which must address physical memory) happy if
   * they require several consecutive pages.
   */
  for (i = pages_initialized; i > 0; i--) {
    if(fill) fillPage(....);
    vm_page_release_startup(&vm_pages[i - 1]);
  }
  ....
}
```

PVS\-Studio warning: V705 CWE\-691 It is possible that 'else' block was forgotten or commented out, thus altering the program's operation logics\. vm\_resident\.c 1248

Perhaps there is no error here\. However, I'm very confused by the keyword _else_\. The code is formatted in such a way as if the loop is always executed\. Actually the loop is executed only when the condition  _\(2 \=\= vm\_himemory\_mode\)_ is false\.

## Conclusion

In the macOS world a new powerful static code [PVS\-Studio](https://pvs-studio.com/en/pvs-studio/) analyzer appeared that is able to detect errors and potential vulnerabilities in C, and C\+\+\. I invite everyone to try out our analyzer on your projects and to assess its abilities\. 

Thanks for your attention and don't forget to share the information with colleagues that PVS\-Studio is now available for macOS\.