Skip to content

Commit

Permalink
Merge pull request #5303 from brong/ubsan-fixes
Browse files Browse the repository at this point in the history 
Ubsan fixes
  • Loading branch information
@brong
brong authored Feb 20, 2025
2 parents ada9926 + c14e806 commit 1842e53
Show file tree
Hide file tree
Showing 35 changed files with 124 additions and 78 deletions.
6 changes: 6 additions & 0 deletions cassandane/Cassandane/Cyrus/Info.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,12 @@ sub test_proc_crashed_services
unlink $cores[0];
}

# sanitizers might complain about the SEGV
my $ubsan_logdir = $self->{instance}->_sanitizer_log_dir("ubsan");
unlink("$ubsan_logdir/ubsan.$pid");
my $asan_logdir = $self->{instance}->_sanitizer_log_dir("ubsan");
unlink("$asan_logdir/asan.$pid");

@output = $self->{instance}->run_cyr_info('proc');
$self->assert_num_equals(scalar @pids, scalar @output);
}
Expand Down
2 changes: 1 addition & 1 deletion cassandane/Cassandane/Instance.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1416,7 +1416,7 @@ sub _sanitizer_log_dir()
{
my ($self, $sanitizer) = @_;

my $san_logdir = $self->{basedir} . "/${sanitizer}logs/";
my $san_logdir = $self->{basedir} . "${sanitizer}logs/";
mkpath $san_logdir
unless ( -d $san_logdir );

Expand Down
4 changes: 2 additions & 2 deletions imap/append.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1541,7 +1541,7 @@ EXPORTED int append_copy(struct mailbox *mailbox, struct appendstate *as,

for (userflag = 0; userflag < MAX_USER_FLAGS; userflag++) {
bit32 flagmask = src_user_flags[userflag/32];
if (mailbox->h.flagname[userflag] && (flagmask & (1<<(userflag&31)))) {
if (mailbox->h.flagname[userflag] && (flagmask & (1U<<(userflag&31)))) {
int num;
r = mailbox_user_flag(as->mailbox, mailbox->h.flagname[userflag], &num, 1);
if (r)
Expand All @@ -1554,7 +1554,7 @@ EXPORTED int append_copy(struct mailbox *mailbox, struct appendstate *as,
src_uid,
error_message(r));
else
dst_user_flags[num/32] |= 1<<(num&31);
dst_user_flags[num/32] |= 1U<<(num&31);
}
}

Expand Down
2 changes: 1 addition & 1 deletion imap/attachextract.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ static int extractor_httpreq(struct extractor_ctx *ext,
"Accept: text/plain\r\n"
"X-Truncate-Length: " SIZE_T_FMT "\r\n",
method, url, HTTP_VERSION,
(int) hostlen, ext->be->hostname, CYRUS_VERSION,
(int) hostlen, ext->hostname, CYRUS_VERSION,
attachextract_idle_timeout, config_search_maxsize);

if (req_body) {
Expand Down
2 changes: 1 addition & 1 deletion imap/carddav_db.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1367,7 +1367,7 @@ EXPORTED int carddav_remove(struct mailbox *mailbox,

if (!r) r = mailbox_find_index_record(mailbox, olduid, &oldrecord);
if (!r && !(oldrecord.internal_flags & FLAG_INTERNAL_EXPUNGED)) {
if (isreplace) oldrecord.user_flags[userflag/32] |= 1<<(userflag&31);
if (isreplace) oldrecord.user_flags[userflag/32] |= 1U<<(userflag&31);
oldrecord.internal_flags |= FLAG_INTERNAL_EXPUNGED;

r = mailbox_rewrite_index_record(mailbox, &oldrecord);
Expand Down
11 changes: 11 additions & 0 deletions imap/conversations.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2060,6 +2060,12 @@ static int _guid_filter_p(void *rock,
return 0; // no match
}

// scratch space to give aligned reads of GUID data
union cdata_u {
bit64 b;
char s[32];
};

static int _guid_cb(void *rock,
const char *key,
size_t keylen,
Expand Down Expand Up @@ -2097,6 +2103,7 @@ static int _guid_cb(void *rock,
uint32_t internal_flags = 0;
time_t internaldate = 0;
char version = 0;
union cdata_u scratch;
if (datalen >= 16) {
const char *p = data;

Expand All @@ -2114,6 +2121,8 @@ static int _guid_cb(void *rock,
case 1: /* OLD - byname version */
case 2: /* original byid version (no basecid) */
/* cid */
memcpy(scratch.s, p, 24);
p = scratch.s;
cid = ntohll(*((bit64*)p));
p += 8;
/* system_flags */
Expand All @@ -2128,6 +2137,8 @@ static int _guid_cb(void *rock,
break;

default:
memcpy(scratch.s, p, 32);
p = scratch.s;
/* cid */
cid = ntohll(*((bit64*)p));
p += 8;
Expand Down
6 changes: 2 additions & 4 deletions imap/ctl_conversationsdb.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -753,11 +753,9 @@ static int fix_modseqs(struct conversations_state *a,
if (ca.key[0] == 'G') {
// basecid might be different on the old record, so if they're both v3, then copy the data over
if (ca.datalen >= 33 && cb.datalen >= 33 && ca.data[0] == (char)0x83 && cb.data[0] == (char)0x83) {
conversation_id_t basea = ntohll(*((bit64*)(ca.data+25)));
conversation_id_t baseb = ntohll(*((bit64*)(cb.data+25)));
if (basea != baseb && cb.datalen < 80) {
if (memcmp(ca.data+25, cb.data+25, 8) && cb.datalen < 80) {
memcpy(buf, cb.data, cb.datalen);
*((uint64_t *)(buf + 25)) = htonll(basea);
memcpy(buf+25, ca.data+25, 8);
r = cyrusdb_store(b->db, cb.key, cb.keylen, buf, cb.datalen, &b->txn);
if (r) {
fprintf(stderr, "Failed to store conversations "
Expand Down
2 changes: 1 addition & 1 deletion imap/cyr_expire.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@ static int expunge_userflags(struct mailbox *mailbox, struct expire_rock *erock)
int r;

for (i = 0; i < MAX_USER_FLAGS; i++) {
if (erock->userflags[i/32] & 1<<(i&31))
if (erock->userflags[i/32] & 1U<<(i&31))
continue;
if (!mailbox->h.flagname[i])
continue;
Expand Down
20 changes: 12 additions & 8 deletions imap/http_caldav.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2307,7 +2307,8 @@ static int list_calendars(struct transaction_t *txn)
lrock.scheddefault = NULL;

/* Sort calendars by displayname */
qsort(lrock.cal, lrock.len, sizeof(struct cal_info), &cal_compare);
if (lrock.len)
qsort(lrock.cal, lrock.len, sizeof(struct cal_info), &cal_compare);
charset_t utf8 = charset_lookupname("utf-8");
buf_printf_markup(body, level, "<thead>");
buf_printf_markup(body, level, "<tr><th colspan='2'>Name</th><th colspan='2'>Description</th><th>Color</th><th>Order</th><th>Components</th><th>WebCAL link</th><th>HTTPS link</th><th>Actions</th><th>Public</th><th>Transparent</th></tr>");
Expand Down Expand Up @@ -5194,7 +5195,7 @@ static int propfind_restype(const xmlChar *name, xmlNsPtr ns,
return 0;
}

#define PROP_NOVALUE (1<<31)
#define PROP_NOVALUE (1U<<31)

static struct partial_comp_t *parse_partial_comp(xmlNodePtr node)
{
Expand Down Expand Up @@ -7605,8 +7606,9 @@ static void combine_vavailability(struct freebusy_filter *fbfilter)
memset(&availfilter, 0, sizeof(struct freebusy_filter));

/* Sort VAVAILABILITY periods by priority and start time */
qsort(vavail->vav, vavail->len,
sizeof(struct vavailability), compare_vavail);
if (vavail->len)
qsort(vavail->vav, vavail->len,
sizeof(struct vavailability), compare_vavail);

/* Maintain a linked list of remaining time ranges
* to be filled in by lower priority VAV components
Expand Down Expand Up @@ -7746,8 +7748,9 @@ HIDDEN icalcomponent *busytime_to_ical(struct freebusy_filter *fbfilter,
if (vavail->len) combine_vavailability(fbfilter);

/* Sort busytime periods by type and start/end times for coalescing */
qsort(freebusy->fb, freebusy->len,
sizeof(struct freebusy), compare_freebusy_with_type);
if (freebusy->len)
qsort(freebusy->fb, freebusy->len,
sizeof(struct freebusy), compare_freebusy_with_type);

/* Coalesce busytime periods of same type into one */
for (n = 0; n + 1 < freebusy->len; n++) {
Expand Down Expand Up @@ -7800,8 +7803,9 @@ HIDDEN icalcomponent *busytime_to_ical(struct freebusy_filter *fbfilter,
}

/* Sort busytime periods by start/end times for addition to VFREEBUSY */
qsort(freebusy->fb, freebusy->len,
sizeof(struct freebusy), compare_freebusy);
if (freebusy->len)
qsort(freebusy->fb, freebusy->len,
sizeof(struct freebusy), compare_freebusy);

/* Construct iCalendar object with VFREEBUSY component */
ical = icalcomponent_vanew(ICAL_VCALENDAR_COMPONENT,
Expand Down
8 changes: 4 additions & 4 deletions imap/http_dav.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3199,14 +3199,14 @@ static int proppatch_toresource(xmlNodePtr prop, unsigned set,
int isset;
r = mailbox_user_flag(pctx->mailbox, (const char *)prop->name, &userflag, 1);
if (r) goto done;
isset = pctx->record->user_flags[userflag/32] & (1<<userflag%31);
isset = pctx->record->user_flags[userflag/32] & (1U<<(userflag&31));
if (set) {
if (isset) goto done;
pctx->record->user_flags[userflag/32] |= (1<<userflag%31);
pctx->record->user_flags[userflag/32] |= (1U<<(userflag&31));
}
else {
if (!isset) goto done;
pctx->record->user_flags[userflag/32] &= ~(1<<userflag%31);
pctx->record->user_flags[userflag/32] &= ~(1U<<(userflag&31));
}
r = mailbox_rewrite_index_record(pctx->mailbox, pctx->record);
goto done;
Expand Down Expand Up @@ -3278,7 +3278,7 @@ static int propfind_fromresource(const xmlChar *name, xmlNsPtr ns,
int isset;
r = mailbox_user_flag(fctx->mailbox, (const char *)name, &userflag, 0);
if (r) goto done;
isset = fctx->record->user_flags[userflag/32] & (1<<userflag%31);
isset = fctx->record->user_flags[userflag/32] & (1U<<(userflag&31));
if (isset)
buf_setcstr(&attrib, "1");
goto done;
Expand Down
2 changes: 1 addition & 1 deletion imap/httpd.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3748,7 +3748,7 @@ EXPORTED void write_body(long code, struct transaction_t *txn,
}

/* Output data */
txn->conn->resp_body_chunk(txn, buf + offset, outlen, last_chunk, &ctx);
txn->conn->resp_body_chunk(txn, buf ? buf + offset : NULL, outlen, last_chunk, &ctx);
}


Expand Down
2 changes: 1 addition & 1 deletion imap/httpd.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ enum {

ALLOW_READONLY = (1<<30),/* Allow "unsafe" methods when readonly */

ALLOW_ISCHEDULE = (1<<31) /* iSchedule specific methods/features */
ALLOW_ISCHEDULE = (1U<<31) /* iSchedule specific methods/features */
};

#define ALLOW_READ_MASK ~(ALLOW_POST|ALLOW_WRITE|ALLOW_DELETE|ALLOW_PATCH\
Expand Down
2 changes: 1 addition & 1 deletion imap/imapd.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -476,7 +476,7 @@ enum {
/* non-standard - track for possible deprecation */
CB_ANNOTATE = (1<<29), /* GET/SETANNOTATION or FETCH ANNOTATION */
CB_XCONV = (1<<30), /* STATUS XCONV* */
CB_XLIST = (1<<31), /* XLIST */
CB_XLIST = (1U<<31), /* XLIST */
};

#endif /* INCLUDED_IMAPD_H */
15 changes: 8 additions & 7 deletions imap/index.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1379,7 +1379,7 @@ EXPORTED int index_store(struct index_state *state, const char *sequence,
for (i = 0; i < flags->count ; i++) {
r = mailbox_user_flag(mailbox, flags->data[i], &userflag, 1);
if (r) goto out;
storeargs->user_flags[userflag/32] |= 1<<(userflag&31);
storeargs->user_flags[userflag/32] |= 1U<<(userflag&31);
}

storeargs->update_time = time((time_t *)0);
Expand Down Expand Up @@ -3317,7 +3317,7 @@ static int index_appendremote(struct index_state *state, uint32_t msgno,
if ((flag & 31) == 0) {
flagmask = record.user_flags[flag/32];
}
if (state->flagname[flag] && (flagmask & (1<<(flag & 31)))) {
if (state->flagname[flag] && (flagmask & (1U<<(flag&31)))) {
prot_printf(pout, "%c%s", sepchar, state->flagname[flag]);
sepchar = ' ';
}
Expand Down Expand Up @@ -4284,7 +4284,7 @@ static void index_fetchflags(struct index_state *state,
if ((flag & 31) == 0) {
flagmask = im->user_flags[flag/32];
}
if (state->flagname[flag] && (flagmask & (1<<(flag & 31)))) {
if (state->flagname[flag] && (flagmask & (1U<<(flag&31)))) {
prot_printf(state->out, "%c%s", sepchar, state->flagname[flag]);
sepchar = ' ';
}
Expand Down Expand Up @@ -6436,14 +6436,14 @@ MsgData **index_msgdata_load(struct index_state *state,
case SORT_HASFLAG: {
const char *name = sortcrit[j].args.flag.name;
if (mailbox_record_hasflag(mailbox, &record, name))
cur->hasflag |= (1<<j);
cur->hasflag |= (1U<<j);
break;
}
case SORT_HASCONVFLAG: {
int idx = preload[j];
/* flag exists in the conversation at all */
if (idx >= 0 && conv.counts[idx] > 0 && j < 31)
cur->hasflag |= (1<<j);
cur->hasflag |= (1U<<j);
break;
}
case SORT_CONVEXISTS:
Expand Down Expand Up @@ -6842,8 +6842,8 @@ static int index_sort_compare(MsgData *md1, MsgData *md2,
case SORT_HASFLAG:
case SORT_HASCONVFLAG:
if (i < 31)
ret = numcmp(md1->hasflag & (1<<i),
md2->hasflag & (1<<i));
ret = numcmp(md1->hasflag & (1U<<i),
md2->hasflag & (1U<<i));
break;
case SORT_FOLDER:
if (md1->folder && md2->folder)
Expand Down Expand Up @@ -7053,6 +7053,7 @@ static int index_sort_compare_reverse_flagged(const void *v1, const void *v2)

void index_msgdata_sort(MsgData **msgdata, int n, const struct sortcrit *sortcrit)
{
if (!n) return;
if (sortcrit_is_uid(sortcrit)) {
qsort(msgdata, n, sizeof(MsgData *), index_sort_compare_uid);
}
Expand Down
8 changes: 4 additions & 4 deletions imap/jmap_backup.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -606,7 +606,7 @@ static int destroy_resource(message_t *msg, jmap_req_t *req,
if (is_replaced) {
int userflag;
r = mailbox_user_flag(mailbox, DFLAG_UNBIND, &userflag, 1);
newrecord.user_flags[userflag/32] |= 1<<(userflag&31);
newrecord.user_flags[userflag/32] |= 1U<<(userflag&31);
}

if (!r) {
Expand Down Expand Up @@ -1661,7 +1661,7 @@ static int restore_message_list_cb(const mbentry_t *mbentry, void *rock)

/* Add message to plan for mailbox */
if (!(rrock->jrestore->mode & DRY_RUN) && userflag >= 0
&& (record->user_flags[userflag/32] & (1<<userflag%31))) {
&& (record->user_flags[userflag/32] & (1U<<userflag&31))) {
struct mailbox_plan *plan =
hash_lookup(mailbox_name(mailbox), mrock->mailboxes);
if (!plan) {
Expand Down Expand Up @@ -2068,13 +2068,13 @@ static void restore_mailbox_cb(const char *mboxname, void *data, void *rock)

message_set_from_record(mailbox, &record, msg);
if (!(rrock->jrestore->mode & DRY_RUN)) {
if (record.user_flags[userflag/32] & (1<<userflag%31)) {
if (record.user_flags[userflag/32] & (1U<<(userflag&31))) {
syslog(log_level,
"UID %u: removing $restored flag (%d)", record.uid, userflag);
struct index_record newrecord;
/* copy the existing index_record */
memcpy(&newrecord, &record, sizeof(struct index_record));
newrecord.user_flags[userflag/32] &= ~(1<<userflag%31);
newrecord.user_flags[userflag/32] &= ~(1U<<(userflag&31));
r = mailbox_rewrite_index_record(mailbox, &newrecord);
if (r) {
syslog(LOG_ERR,
Expand Down
4 changes: 2 additions & 2 deletions imap/jmap_calendar.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6954,7 +6954,7 @@ static int eventquery_textsearch_run(jmap_req_t *req,
if (r) goto done;
}

if (!expandrecur) {
if (!expandrecur && matches->count) {
struct eventquery_cmp_rock rock = { sort, nsort };
cyr_qsort_r(matches->data, matches->count, sizeof(void*),
(int(*)(const void*, const void*, void*))eventquery_cmp, &rock);
Expand Down Expand Up @@ -9960,7 +9960,7 @@ static void notifsearch_run(const char *userid,
}
mailbox_iter_done(&iter);

if (search->sort) {
if (search->sort && entries->count) {
cyr_qsort_r(entries->data, entries->count,
sizeof(struct notifsearch_entry),
(int(*)(const void*, const void*, void*))search->sort,
Expand Down
8 changes: 4 additions & 4 deletions imap/jmap_mail.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6567,7 +6567,7 @@ static int _email_keywords_add_msgrecord(struct email_keywords *keywords,
struct buf buf = BUF_INITIALIZER;
int i;
for (i = 0 ; i < MAX_USER_FLAGS ; i++) {
if (mbox->h.flagname[i] && (user_flags[i/32] & 1<<(i&31))) {
if (mbox->h.flagname[i] && (user_flags[i/32] & 1U<<(i&31))) {
buf_setcstr(&buf, mbox->h.flagname[i]);
_email_keywords_add_keyword(keywords, buf_lcase(&buf));
}
Expand Down Expand Up @@ -11622,9 +11622,9 @@ static int _email_setflags(json_t *keywords, int patch_keywords,
r = mailbox_user_flag(mbox, keyword, &userflag, 1);
if (r) goto done;
if (jval == json_true())
new_user_flags[userflag/32] |= 1<<(userflag&31);
new_user_flags[userflag/32] |= 1U<<(userflag&31);
else
new_user_flags[userflag/32] &= ~(1<<(userflag&31));
new_user_flags[userflag/32] &= ~(1U<<(userflag&31));
}
}
if (!patch_keywords && del_seen_uids) {
Expand Down Expand Up @@ -13760,7 +13760,7 @@ static time_t _email_import_parse_received_at(const char *blob, size_t blob_len)
if (hdr)
hdr = strchr(hdr + 1, ':');
xzfree(val);
} while (!received_at && hdr++);
} while (!received_at && hdr && hdr++); // only do side effect ++ if non-NULL
}

if (!received_at && !message_get_field(msg, "Date", format, &buf)) {
Expand Down
2 changes: 1 addition & 1 deletion imap/jmap_notes.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1017,7 +1017,7 @@ static int jmap_note_changes(jmap_req_t *req)
if (record->internal_flags & FLAG_INTERNAL_EXPUNGED) {
/* Skip any notes that have been replaced by an update */
if (userflag >= 0 &&
record->user_flags[userflag/32] & (1<<(userflag & 31))) continue;
record->user_flags[userflag/32] & (1U<<(userflag&31))) continue;

/* Skip any notes created AND deleted since modseq */
if (record->createdmodseq > changes.since_modseq) continue;
Expand Down
2 changes: 1 addition & 1 deletion imap/jmap_quota.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -913,7 +913,7 @@ static int jmap_quota_query(jmap_req_t *req)
hash_enumerate(&qrock.quotas, &filter_cb, &frock);

/* Sort results */
if (arrayu64_size(&sortcrit)) {
if (arrayu64_size(&sortcrit) && frock.matches.count) {
cyr_qsort_r(frock.matches.data, frock.matches.count,
sizeof(void *), &quota_cmp, &sortcrit);
}
Expand Down
Loading

0 comments on commit 1842e53

Please sign in to comment.