A principios de este año, Apple lanzó el código fuente de los componentes del sistema macOS 11.0, Big Sur, incluido XNU, el núcleo del sistema operativo macOS. Hace un par de años, PVS-Studio ya verificó el código fuente del kernel en relación con el lanzamiento del analizador para macOS. Pasó bastante tiempo y se lanzó una nueva versión del código fuente del kernel. ¿Por qué no volver a probar?
¿Qué es este proyecto, de Apple y de código abierto?
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?.