At the very beginning of this year, Apple released the source code of macOS 11.0 system components - Big Sur, including XNU - the kernel of the macOS operating system. A couple of years ago, the source code of the kernel was already checked by PVS-Studio in connection with the release of the analyzer for macOS. Quite a long time has passed, and a new release of the kernel source code has been released. Why not retest.
What is this project, Apple and open-source?
XNU β X is Not Unix β Apple OS X. 20 APSL (Apple Public Source License) OC Darwin. Darwin , . , open-source .
, PVS-Studio. : " PVS-Studio macOS: 64 weaknesses Apple XNU Kernel". , . , , . . PVS-Studio :). , , GitHub .
, , , . . , . , XNU, .
. , , . , , . , - . 64!
.
N1, :
int
key_parse(
struct mbuf *m,
struct socket *so)
{
....
if ((m->m_flags & M_PKTHDR) == 0 ||
m->m_pkthdr.len != m->m_pkthdr.len) {
....
goto senderror;
}
....
}
:
, orglen, :
#define PFKEY_UNUNIT64(a) ((a) << 3)
, : orglen, .
, , β N5, - .
assertf β , β .
6 7 . , . PBUF_TYPE_MBUF PBUF_TYPE_MEMORY .
N8, 9, 10 :
, ( xnu-4903.270.47 11 ) -. , . PVS-Studio . , .
11, 12, 13, 14 β 11:
. , - ;) ( , ). , , :
static int
kauth_resolver_getwork(user_addr_t message)
{
struct kauth_resolver_work *workp;
int error;
KAUTH_RESOLVER_LOCK();
error = 0;
while ((workp = TAILQ_FIRST(....)) == NULL) { // <=
thread_t thread = current_thread();
struct uthread *ut = get_bsdthread_info(thread);
ut->uu_save.uus_kauth.message = message;
error = msleep0(....);
KAUTH_RESOLVER_UNLOCK();
/*
* If this is a wakeup from another thread in the resolver
* deregistering it, error out the request-for-work thread
*/
if (!kauth_resolver_identity) {
printf("external resolver died");
error = KAUTH_RESOLVER_FAILED_ERRCODE;
}
return error; //<=
}
return kauth_resolver_getwork2(message);
}
PVS-Studio: V612 An unconditional 'return' within a loop. kern_credential.c 951
, , . , error. , , (workp = TAILQ_FIRST(....)) == NULL, . - if while, . error = msleep0(....) :
error = msleep0(&kauth_resolver_unsubmitted,
kauth_resolver_mtx,
PCATCH,
"GRGetWork",
0,
kauth_resolver_getwork_continue);
kauth_resolver_getwork_continue. , , . if, while.
static int
kauth_resolver_getwork_continue(int result)
{
....
if (TAILQ_FIRST(&kauth_resolver_unsubmitted) == NULL) {
....
return error;
}
....
}
, . ( kauth_resolver_getwork_continue), , , , . , while . , , , .
. N40. :
PVS-Studio: 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
, , :
, 62 , . .
63 64 , . , , .
, XNU PVS-Studio. , , . PVS-Studio , .
cloc 1346 *.c , 1822 /C++ 225 *.cpp .
.
N1
void
pe_identify_machine(__unused boot_args *args)
{
....
// Start with default values.
gPEClockFrequencyInfo.timebase_frequency_hz = 1000000000;
gPEClockFrequencyInfo.bus_frequency_hz = 100000000;
....
gPEClockFrequencyInfo.dec_clock_rate_hz =
gPEClockFrequencyInfo.timebase_frequency_hz;
gPEClockFrequencyInfo.bus_clock_rate_hz =
gPEClockFrequencyInfo.bus_frequency_hz;
....
gPEClockFrequencyInfo.bus_to_dec_rate_den =
gPEClockFrequencyInfo.bus_clock_rate_hz /
gPEClockFrequencyInfo.dec_clock_rate_hz;
}
PVS-Studio: V1064 The 'gPEClockFrequencyInfo.bus_clock_rate_hz' operand of integer division is less than the 'gPEClockFrequencyInfo.dec_clock_rate_hz' one. The result will always be zero. pe_identify_machine.c 72
:
extern clock_frequency_info_t gPEClockFrequencyInfo;
struct clock_frequency_info_t {
unsigned long bus_clock_rate_hz;
unsigned long dec_clock_rate_hz;
unsigned long bus_to_dec_rate_den;
unsigned long long bus_frequency_hz;
unsigned long timebase_frequency_hz;
....
};
gPEClockFrequencyInfo.bus_clock_rate_hz, , 100000000, - gPEClockFrequencyInfo.dec_clock_rate_hz 1000000000. . , gPEClockFrequencyInfo.bus_to_dec_rate_den 0.
bus_to_dec_rate_den, . , , 0. .
N2
void
sdt_early_init( void )
{
....
if (MH_MAGIC_KERNEL != _mh_execute_header.magic) {
....
} else {
....
for (....) {
const char *funcname;
unsigned long best; //<=
....
funcname = "<unknown>";
for (i = 0; i < orig_st->nsyms; i++) {
char *jname = strings + sym[i].n_un.n_strx;
....
if ((unsigned long)sym[i].n_value > best) { //<=
best = (unsigned long)sym[i].n_value;
funcname = jname;
}
}
.....
}
}
PVS-Studio: V614 Uninitialized variable 'best' used. sdt.c 572
, . best, , . . best, . , , .
N3
int
cdevsw_isfree(int index)
{
struct cdevsw * devsw;
if (index < 0) {
if (index == -1) {
index = 0;
} else {
index = -index;
}
devsw = &cdevsw[index];
for (; index < nchrdev; index++, devsw++) {
if (memcmp(....) == 0) {
break;
}
}
}
if (index < 0 || index >= nchrdev) {
return -1;
}
....
return index;
}
PVS-Studio: V560 A part of conditional expression is always false: index < 0. bsd_stubs.c:236
, . index . , . if , index , .
, , - . .
N4
int
nfs_vinvalbuf_internal(....)
{
struct nfsbuf *bp;
....
off_t end = ....;
/* check for any dirty data before the EOF */
if ((bp->nb_dirtyend > 0) && (bp->nb_dirtyoff < end))
{
/* clip dirty range to EOF */
if (bp->nb_dirtyend > end)
{
bp->nb_dirtyend = end;
if (bp->nb_dirtyoff >= bp->nb_dirtyend) //<=
{
bp->nb_dirtyoff = bp->nb_dirtyend = 0;
}
}
if ((bp->nb_dirtyend > 0) && (bp->nb_dirtyoff < end)) //<=
{
....
}
}
....
}
PVS-Studio:
- V547 Expression 'bp->nb_dirtyoff >= bp->nb_dirtyend' is always false. nfs_bio.c 3858
- V560 A part of conditional expression is always true: (bp->nb_dirtyoff < end). nfs_bio.c 3862
. , .
. , nb_dirtyoff nb_dirtyend. . if (bp->nb_dirtyend > 0) && (bp->nb_dirtyoff < end) bp->nb_dirtyend > end. bp->nb_dirtyend = end.
bp->nb_dirtyoff >= bp->nb_dirtyend false?
. , nb_dirtyoff , end, nb_dirtyend end. nb_dirtyend , nb_dirtyoff. bp->nb_dirtyoff = bp->nb_dirtyend = 0 .
:
if ((bp->nb_dirtyend > 0) && (bp->nb_dirtyoff < end)) {
/* clip dirty range to EOF */
if (bp->nb_dirtyend > end) {
bp->nb_dirtyend = end;
if (bp->nb_dirtyoff >= bp->nb_dirtyend) { //<=
bp->nb_dirtyoff = bp->nb_dirtyend = 0;
}
}
}
:
if ((bp->nb_dirtyend > 0) && (bp->nb_dirtyoff < end)) {
if (bp->nb_dirtyend > end) {
bp->nb_dirtyend = end;
}
}
.
if, .
if ((bp->nb_dirtyend > 0) && (bp->nb_dirtyoff < end))
, . bp->nb_dirtyoff < end - .
N5
tcp_output(struct tcpcb *tp)
{
....
if (isipv6) {
....
if (len + optlen) {
....
}
} else {
....
if (len + optlen) {
....
}
}
....
}
PVS-Studio: V793 It is odd that the result of the 'len + optlen' statement is a part of the condition. Perhaps, this statement should have been compared with something else.
. . , . , , , 0 , .
, , , :
if (len + optlen + ipoptlen > tp->t_maxopd) {
....
}
, , , if', , .
, , 16 , 2268 ! ;)
:
V793 It is odd that the result of the 'len + optlen' statement is a part of the condition. Perhaps, this statement should have been compared with something else.
N6
int
ttyinput(int c, struct tty *tp)
{
....
if (tp->t_rawq.c_cc + tp->t_canq.c_cc) {
....
}
PVS-Studio: V793 It is odd that the result of the 'tp->t_rawq.c_cc + tp->t_canq.c_cc' statement is a part of the condition. Perhaps, this statement should have been compared with something else. tty.c 568
. , , :
if ( tp->t_rawq.c_cc + tp->t_canq.c_cc > I_HIGH_WATER β 3 // <=
&& ....) {
....
}
, , . if. - , ;)
N7
errno_t
mbuf_adjustlen(mbuf_t m, int amount)
{
/* Verify m_len will be valid after adding amount */
if (amount > 0) {
int used = (size_t)mbuf_data(m)
- (size_t)mbuf_datastart(m)
+ m->m_len;
if ((size_t)(amount + used) > mbuf_maxlen(m)) {
....
}
....
return 0;
}
PVS-Studio: V1028 Possible overflow. Consider casting operands of the 'amount + used' operator to the 'size_t' type, not the result. kpi_mbuf.c
, . size_t , , size_t . , mbuf_maxlen(m) , size_t. - , :
if ((size_t)amount + used > mbuf_maxlen(m))
, .
- V1028 Possible overflow. Consider casting operands, not the result. vm_compressor_pager.c 1165
- V1028 Possible overflow. Consider casting operands, not the result. vm_compressor_pager.c 1131
- V1028 Possible overflow. Consider casting operands, not the result. audit_worker.c 241
- V1028 Possible overflow. Consider casting operands of the '((u_int32_t) slp * hz) + 999999' operator to the 'long' type, not the result. tty.c 2199
N8
int
fdavail(proc_t p, int n)
{
....
char *flags;
int i;
int lim;
....
lim = (int)MIN(....);
if ((i = lim - fdp->fd_nfiles) > 0 && (n -= i) <= 0) //<=
{
return 1;
}
....
for (....)
{
if (*fpp == NULL && !(*flags & UF_RESERVED) && --n <= 0)
{
return 1;
}
}
return 0;
}
PVS-Studio: V1019 Compound assignment expression 'n -= i' is used inside condition. kern_descrip.c_99 3916
, , . , , , :
i = lim - fdp->fd_nfiles;
if (i > 0)
{
n -= i;
if(n <= 0)
return 1;
}
, . Godbolt (Compiler Explorer), , , PVS-Studio. .
, if, n . , . :
i = lim - fdp->fd_nfiles;
if (i > 0) {
if(n β i <= 0)
return 1;
}
, , n. (n -= i) <= 0 , n. , , .
N9
static errno_t
vsock_put_message_listening(struct vsockpcb *pcb,
enum vsock_operation op,
struct vsock_address src,
struct vsock_address dst)
{
switch (op)
{
case VSOCK_REQUEST:
....
if (....)
{
vsock_pcb_safe_reset_address(pcb, dst, src);
....
}
....
done:
....
break;
case VSOCK_RESET:
error = vsock_pcb_safe_reset_address(pcb, dst, src);
break;
default:
vsock_pcb_safe_reset_address(pcb, dst, src);
....
break;
}
return error;
}
PVS-Studio: V764 Possible incorrect order of arguments passed to 'vsock_pcb_safe_reset_address' function: 'dst' and 'src'. vsock_domain.c 549
. , :
static errno_t
vsock_pcb_safe_reset_address(struct vsockpcb *pcb,
struct vsock_address src,
struct vsock_address dst)
.
:
- V764 Possible incorrect order of arguments passed to 'vsock_pcb_safe_reset_address' function: 'dst' and 'src'. vsock_domain.c 587
- V764 Possible incorrect order of arguments passed to 'vsock_pcb_safe_reset_address' function: 'dst' and 'src'. vsock_domain.c 590
N10
int
ifclassq_tbr_set(struct ifclassq *ifq, ....)
{
struct tb_regulator *tbr;
....
tbr = &ifq->ifcq_tbr;
....
tbr->tbr_rate = TBR_SCALE(rate / 8) / machclk_freq;
....
tbr->tbr_last = read_machclk();
if ( tbr->tbr_rate > 0 //<=
&& (ifp->if_flags & IFF_UP))
{
....
} else {
....
}
....
return 0;
}
PVS-Studio: V1051 Consider checking for misprints. It's possible that the 'tbr->tbr_last' should be checked here. classq_subr.c 685
, , . . , tbr_rate 35 . tbr_last, , . , tbr_rate.
N11
void
audit_arg_mac_string(struct kaudit_record *ar, ....)
{
if (ar->k_ar.ar_arg_mac_string == NULL)
{
ar->k_ar.ar_arg_mac_string = kheap_alloc(....);
}
....
if (ar->k_ar.ar_arg_mac_string == NULL)
{
if (ar->k_ar.ar_arg_mac_string == NULL) // <=
{
return;
}
}
....
}
PVS-Studio: V571 Recurring check. The 'if (ar->k_ar.ar_arg_mac_string == NULL)' condition was already verified in line 245. audit_mac.c 246
PVS-Studio: V547 Expression 'ar->k_ar.ar_arg_mac_string == NULL' is always true. audit_mac.c 246
.
, if . : , :
/*
* XXX This should be a rare event.
* If kheap_alloc() returns NULL,
* the system is low on kernel virtual memory. To be
* consistent with the rest of audit, just return
* (may need to panic if required to for audit).
*/
, . . , , .
, - . , .
N12
int
utf8_encodestr(....)
{
u_int16_t ucs_ch;
int swapbytes = ....;
....
ucs_ch = swapbytes ? OSSwapInt16(*ucsp++) : *ucsp++;
....
}
PVS-Studio: V567 Undefined behavior. The 'ucsp' variable is modified while being used twice between sequence points. vfs_utfconv.c 298
β . , " C++ ". . .
. , , . , OSSwapInt16(*ucsp++).
, , .i , . :
ucs_ch = swapbytes
? ( (__uint16_t)(__builtin_constant_p(*ucsp++)
? ((__uint16_t)( (((__uint16_t)(*ucsp++) & 0xff00U) >> 8)
| (((__uint16_t)(*ucsp++) & 0x00ffU) << 8)))
: _OSSwapInt16(*ucsp++)))
: *ucsp++;
:
(((__uint16_t)(*ucsp++) & 0xff00U) >> 8)
| (((__uint16_t)(*ucsp++) & 0x00ffU) << 8)
. , | , *uscp .
V567 PVS-Studio . , , .
! . , , , *ucsp . , , . . - . . , .
N13
struct pf_status pf_status;
int
pf_insert_state(struct pf_state *s, ....)
{
....
if (....) {
s->id = htobe64(pf_status.stateid++);
....
}
....
}
PVS-Studio: V567 Undefined behavior. The 'pf_status.stateid' variable is modified while being used twice between sequence points. pf.c 1440
. htobe64, :
s->id = (__builtin_constant_p(pf_status.stateid++) ?
((__uint64_t)((((__uint64_t)(pf_status.stateid++) &
0xff00000000000000ULL) >> 56) | (((__uint64_t)(pf_status.stateid++) &
0x00ff000000000000ULL) >> 40) | (((__uint64_t)(pf_status.stateid++) &
0x0000ff0000000000ULL) >> 24) | (((__uint64_t)(pf_status.stateid++) &
0x000000ff00000000ULL) >> 8) | (((__uint64_t)(pf_status.stateid++) &
0x00000000ff000000ULL) << 8) | (((__uint64_t)(pf_status.stateid++) &
0x0000000000ff0000ULL) << 24) | (((__uint64_t)(pf_status.stateid++) &
0x000000000000ff00ULL) << 40) | (((__uint64_t)(pf_status.stateid++) &
0x00000000000000ffULL) << 56))) : _OSSwapInt64(pf_status.stateid++));
, . | & . , pf_status.stateid . .
, -, , :).
:
- V567 Undefined behavior. The 'ip_id' variable is modified while being used twice between sequence points. ip_id.c 186
- V567 Undefined behavior. The 'lp' variable is modified while being used twice between sequence points. nfs_boot.c 505
- V567 Undefined behavior. The 'lp' variable is modified while being used twice between sequence points. nfs_boot.c 497
- V567 Undefined behavior. The 'ip_id' variable is modified while being used twice between sequence points. kdp_udp.c 588
- V567 Undefined behavior. The 'ip_id' variable is modified while being used twice between sequence points. kdp_udp.c 665
- V567 Undefined behavior. The 'ip_id' variable is modified while being used twice between sequence points. kdp_udp.c 1543
N14
__private_extern__ boolean_t
ipsec_send_natt_keepalive(....)
{
....
struct udphdr *uh = (__typeof__(uh))(void *)( (char *)m_mtod(m)
+ sizeof(*ip));
....
if (....)
{
uh->uh_sport = (u_short)sav->natt_encapsulated_src_port;
} else {
uh->uh_sport = htons((u_short)esp_udp_encap_port);
}
uh->uh_sport = htons((u_short)esp_udp_encap_port);
....
}
PVS-Studio: V519 The 'uh->uh_sport' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 4866, 4870. ipsec.c 4870
: uh_sport . if-else , else. if-else , .
N15
static kern_return_t
vm_shared_region_slide_page_v3(vm_offset_t vaddr, ....)
{
....
uint8_t *page_content = (uint8_t *)vaddr;
uint16_t page_entry;
....
uint8_t* rebaseLocation = page_content;
uint64_t delta = page_entry;
do {
rebaseLocation += delta;
uint64_t value;
memcpy(&value, rebaseLocation, sizeof(value));
....
bool isBind = (value & (1ULL << 62)) == 1; // <=
if (isBind) {
return KERN_FAILURE;
}
....
} while (delta != 0);
....
}
PVS-Studio: V547 Expression '(value & (1ULL << 62)) == 1' is always false. vm_shared_region.c 2820
- . isBind. .
63- . & value 0 0x4000000000000000. 1. .
, KERN_FAILURE, , 0x4000000000000000 , . , 1. , :
bool isBind = (value & (1ULL << 62)) != 0;
N16
int
vn_path_package_check(char *path, int pathlen, ....)
{
char *ptr, *end;
int comp = 0;
....
end = path + 1;
while (end < path + pathlen && *end != '\0') {
while (end < path + pathlen && *end == '/' && *end != '\0') {
end++;
}
ptr = end;
while (end < path + pathlen && *end != '/' && *end != '\0') {
end++;
}
....
}
....
}
PVS-Studio: V590 Consider inspecting this expression. The expression is excessive or contains a misprint. vfs_subr.c 3589
. . , , . while. , '/' '\0'. , *end '/', '\0'.
while . - , , . , while, '/'. , - .
, . , XNU . Clang Static Analyzer. - . , .
, , , , .
, PVS-Studio , . , , , , , , . , Qt6.
, : Victoria Khanieva. MacOS Kernel, Is This Apple Rotten?.