Skip to content
Merged
14 changes: 12 additions & 2 deletions arch/x86/core/acpi.c
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,12 @@ void *z_acpi_find_table(uint32_t signature)

uint32_t *end = (uint32_t *)((char *)rsdt + rsdt->sdt.length);

for (uint32_t *tp = &rsdt->table_ptrs[0]; tp < end; tp++) {
/* Extra indirection required to avoid
* -Waddress-of-packed-member
*/
void *table_ptrs = &rsdt->table_ptrs[0];

for (uint32_t *tp = table_ptrs; tp < end; tp++) {
t_phys = (long)*tp;
z_phys_map(&mapped_tbl, t_phys, sizeof(*t), 0);
t = (void *)mapped_tbl;
Expand Down Expand Up @@ -186,7 +191,12 @@ void *z_acpi_find_table(uint32_t signature)

uint64_t *end = (uint64_t *)((char *)xsdt + xsdt->sdt.length);

for (uint64_t *tp = &xsdt->table_ptrs[0]; tp < end; tp++) {
/* Extra indirection required to avoid
* -Waddress-of-packed-member
*/
void *table_ptrs = &xsdt->table_ptrs[0];

for (uint64_t *tp = table_ptrs; tp < end; tp++) {
t_phys = (long)*tp;
z_phys_map(&mapped_tbl, t_phys, sizeof(*t), 0);
t = (void *)mapped_tbl;
Expand Down
8 changes: 0 additions & 8 deletions cmake/compiler/gcc/compiler_flags.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,6 @@ check_set_compiler_property(APPEND PROPERTY warning_base -Wpointer-arith)
# not portable
check_set_compiler_property(APPEND PROPERTY warning_base -Wexpansion-to-defined)

if(CMAKE_CXX_COMPILER_VERSION VERSION_GREATER "9.1.0")
set_compiler_property(APPEND PROPERTY warning_base
# FIXME: Remove once #16587 is fixed
-Wno-address-of-packed-member
)
endif()


# GCC options for warning levels 1, 2, 3, when using `-DW=[1|2|3]`
set_compiler_property(PROPERTY warning_dw_1
-Waggregate-return
Expand Down
5 changes: 4 additions & 1 deletion drivers/gpio/gpio_sx1509b.c
Original file line number Diff line number Diff line change
Expand Up @@ -453,7 +453,10 @@ static int port_write(const struct device *dev,

const struct sx1509b_config *cfg = dev->config;
struct sx1509b_drv_data *drv_data = dev->data;
uint16_t *outp = &drv_data->pin_state.data;
void *data = &drv_data->pin_state.data;
uint16_t *outp = data;

__ASSERT_NO_MSG(IS_PTR_ALIGNED(data, uint16_t));

k_sem_take(&drv_data->lock, K_FOREVER);

Expand Down
6 changes: 2 additions & 4 deletions drivers/sensor/bmi160/bmi160.c
Original file line number Diff line number Diff line change
Expand Up @@ -791,8 +791,7 @@ static inline void bmi160_gyr_channel_get(const struct device *dev,
{
struct bmi160_data *data = to_data(dev);

bmi160_channel_convert(chan, data->scale.gyr,
data->sample.gyr, val);
bmi160_channel_convert(chan, data->scale.gyr, data->sample.gyr, val);
}
#endif

Expand All @@ -803,8 +802,7 @@ static inline void bmi160_acc_channel_get(const struct device *dev,
{
struct bmi160_data *data = to_data(dev);

bmi160_channel_convert(chan, data->scale.acc,
data->sample.acc, val);
bmi160_channel_convert(chan, data->scale.acc, data->sample.acc, val);
}
#endif

Expand Down
2 changes: 1 addition & 1 deletion drivers/sensor/bmi160/bmi160.h
Original file line number Diff line number Diff line change
Expand Up @@ -471,7 +471,7 @@ union bmi160_sample {
#if !defined(CONFIG_BMI160_ACCEL_PMU_SUSPEND)
uint16_t acc[BMI160_AXES];
#endif
} __packed;
};
};

struct bmi160_scale {
Expand Down
3 changes: 3 additions & 0 deletions include/toolchain/common.h
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,9 @@
*/
#define Z_DECL_ALIGN(type) __aligned(__alignof(type)) type

/* Check if a pointer is aligned enough for a particular data type. */
#define IS_PTR_ALIGNED(ptr, type) ((((uintptr_t)ptr) % __alignof(type)) == 0)

/**
* @brief Iterable Sections APIs
* @defgroup iterable_section_apis Iterable Sections APIs
Expand Down
5 changes: 4 additions & 1 deletion kernel/mmu.c
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,10 @@ static void free_page_frame_list_put(struct z_page_frame *pf)
{
PF_ASSERT(pf, z_page_frame_is_available(pf),
"unavailable page put on free list");
sys_slist_append(&free_page_frame_list, &pf->node);
/* The structure is packed, which ensures that this is true */
void *node = pf;

sys_slist_append(&free_page_frame_list, node);
z_free_page_count++;
}

Expand Down
7 changes: 5 additions & 2 deletions kernel/userspace.c
Original file line number Diff line number Diff line change
Expand Up @@ -466,8 +466,11 @@ static void unref_check(struct z_object *ko, uintptr_t index)
sys_bitfield_clear_bit((mem_addr_t)&ko->perms, index);

#ifdef CONFIG_DYNAMIC_OBJECTS
struct dyn_obj *dyn =
CONTAINER_OF(ko, struct dyn_obj, kobj);
void *vko = ko;

struct dyn_obj *dyn = CONTAINER_OF(vko, struct dyn_obj, kobj);
/* TODO: check why this assert hits */
/*__ASSERT(IS_PTR_ALIGNED(dyn, struct dyn_obj), "unaligned z_object");*/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to repeat my comment from elsewhere: this points to some suspicious behavior in the kernel dynamic object allocator. We should probably open a bug to make sure it gets looked at. Given the architecture, it's not illegal to misalign structs, but we shouldn't be doing it without a good reason.

(If I had to make a wild guess, it was written for i386 and 4-byte-aligns everything, so misses half the time for structs with 64 bit pointers.)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, see here: #41062


if ((ko->flags & K_OBJ_FLAG_ALLOC) == 0U) {
goto out;
Expand Down
61 changes: 53 additions & 8 deletions subsys/bluetooth/controller/hci/hci.c
Original file line number Diff line number Diff line change
Expand Up @@ -3728,16 +3728,23 @@ static void le_cis_request(struct pdu_data *pdu_data,
{
struct bt_hci_evt_le_cis_req *sep;
struct node_rx_conn_iso_req *req;
void *node;

if (!(event_mask & BT_EVT_MASK_LE_META_EVENT) ||
!(le_event_mask & BT_EVT_MASK_LE_CIS_REQ)) {
return;
}

req = (void *)pdu_data;

sep = meta_evt(buf, BT_HCI_EVT_LE_CIS_REQ, sizeof(*sep));
sep->acl_handle = sys_cpu_to_le16(node_rx->hdr.handle);

/* Check for pdu field being aligned before accessing CIS established
* event.
*/
node = pdu_data;
LL_ASSERT(IS_PTR_ALIGNED(node, struct node_rx_conn_iso_estab));

req = node;
sep->cis_handle = sys_cpu_to_le16(req->cis_handle);
sep->cig_id = req->cig_id;
sep->cis_id = req->cis_id;
Expand All @@ -3757,6 +3764,7 @@ static void le_cis_established(struct pdu_data *pdu_data,
struct ll_conn_iso_stream *cis;
struct ll_conn_iso_group *cig;
bool is_central;
void *node;

if (!(event_mask & BT_EVT_MASK_LE_META_EVENT) ||
!(le_event_mask & BT_EVT_MASK_LE_CIS_ESTABLISHED)) {
Expand All @@ -3769,10 +3777,16 @@ static void le_cis_established(struct pdu_data *pdu_data,
is_central = cig->lll.role == BT_CONN_ROLE_CENTRAL;
lll_cis_c = is_central ? &lll_cis->tx : &lll_cis->rx;
lll_cis_p = is_central ? &lll_cis->rx : &lll_cis->tx;
est = (void *)pdu_data;

sep = meta_evt(buf, BT_HCI_EVT_LE_CIS_ESTABLISHED, sizeof(*sep));

/* Check for pdu field being aligned before accessing CIS established
* event.
*/
node = pdu_data;
LL_ASSERT(IS_PTR_ALIGNED(node, struct node_rx_conn_iso_estab));

est = node;
sep->status = est->status;
sep->conn_handle = sys_cpu_to_le16(est->cis_handle);
sys_put_le24(cig->sync_delay, sep->cig_sync_delay);
Expand Down Expand Up @@ -6063,6 +6077,7 @@ static void le_per_adv_sync_established(struct pdu_data *pdu_data,
struct bt_hci_evt_le_per_adv_sync_established *sep;
struct ll_scan_set *scan;
struct node_rx_sync *se;
void *node;

if (!(event_mask & BT_EVT_MASK_LE_META_EVENT) ||
!(le_event_mask & BT_EVT_MASK_LE_PER_ADV_SYNC_ESTABLISHED)) {
Expand All @@ -6072,7 +6087,13 @@ static void le_per_adv_sync_established(struct pdu_data *pdu_data,
sep = meta_evt(buf, BT_HCI_EVT_LE_PER_ADV_SYNC_ESTABLISHED,
sizeof(*sep));

se = (void *)pdu_data;
/* Check for pdu field being aligned before accessing sync established
* event.
*/
node = pdu_data;
LL_ASSERT(IS_PTR_ALIGNED(node, struct node_rx_sync));

se = node;
sep->status = se->status;

if (se->status == BT_HCI_ERR_OP_CANCELLED_BY_HOST) {
Expand Down Expand Up @@ -6403,6 +6424,7 @@ static void le_big_sync_established(struct pdu_data *pdu,
struct node_rx_sync_iso *se;
struct lll_sync_iso *lll;
size_t evt_size;
void *node;

if (!(event_mask & BT_EVT_MASK_LE_META_EVENT) ||
!(le_event_mask & BT_EVT_MASK_LE_BIG_SYNC_ESTABLISHED)) {
Expand All @@ -6417,7 +6439,13 @@ static void le_big_sync_established(struct pdu_data *pdu,
sep = meta_evt(buf, BT_HCI_EVT_LE_BIG_SYNC_ESTABLISHED, evt_size);
sep->big_handle = sys_cpu_to_le16(node_rx->hdr.handle);

se = (void *)pdu;
/* Check for pdu field being aligned before accessing ISO sync
* established event.
*/
node = pdu;
LL_ASSERT(IS_PTR_ALIGNED(node, struct node_rx_sync_iso));

se = node;
sep->status = se->status;
if (sep->status) {
return;
Expand Down Expand Up @@ -6612,9 +6640,19 @@ static void le_scan_req_received(struct pdu_data *pdu_data,
static void le_conn_complete(struct pdu_data *pdu_data, uint16_t handle,
struct net_buf *buf)
{
struct node_rx_cc *cc = (void *)pdu_data;
struct bt_hci_evt_le_conn_complete *lecc;
uint8_t status = cc->status;
struct node_rx_cc *cc;
uint8_t status;
void *node;

/* Check for pdu field being aligned before accessing connection
* complete event.
*/
node = pdu_data;
LL_ASSERT(IS_PTR_ALIGNED(node, struct node_rx_cc));

cc = node;
status = cc->status;

#if defined(CONFIG_BT_CTLR_PRIVACY)
if (!status) {
Expand Down Expand Up @@ -6731,6 +6769,7 @@ static void le_conn_update_complete(struct pdu_data *pdu_data, uint16_t handle,
{
struct bt_hci_evt_le_conn_update_complete *sep;
struct node_rx_cu *cu;
void *node;

if (!(event_mask & BT_EVT_MASK_LE_META_EVENT) ||
!(le_event_mask & BT_EVT_MASK_LE_CONN_UPDATE_COMPLETE)) {
Expand All @@ -6739,7 +6778,13 @@ static void le_conn_update_complete(struct pdu_data *pdu_data, uint16_t handle,

sep = meta_evt(buf, BT_HCI_EVT_LE_CONN_UPDATE_COMPLETE, sizeof(*sep));

cu = (void *)pdu_data;
/* Check for pdu field being aligned before accessing connection
* update complete event.
*/
node = pdu_data;
LL_ASSERT(IS_PTR_ALIGNED(node, struct node_rx_cu));

cu = node;
sep->status = cu->status;
sep->handle = sys_cpu_to_le16(handle);
sep->interval = sys_cpu_to_le16(cu->interval);
Expand Down
9 changes: 8 additions & 1 deletion subsys/bluetooth/controller/ll_sw/ull_central.c
Original file line number Diff line number Diff line change
Expand Up @@ -807,6 +807,7 @@ void ull_central_setup(struct node_rx_hdr *rx, struct node_rx_ftr *ftr,
struct ll_conn *conn;
memq_link_t *link;
uint8_t chan_sel;
void *node;

/* Get reference to Tx-ed CONNECT_IND PDU */
pdu_tx = (void *)((struct node_rx_pdu *)rx)->pdu;
Expand All @@ -820,8 +821,14 @@ void ull_central_setup(struct node_rx_hdr *rx, struct node_rx_ftr *ftr,
/* This is the chan sel bit from the received adv pdu */
chan_sel = pdu_tx->chan_sel;

/* Check for pdu field being aligned before populating connection
* complete event.
*/
node = pdu_tx;
LL_ASSERT(IS_PTR_ALIGNED(node, struct node_rx_cc));

/* Populate the fields required for connection complete event */
cc = (void *)pdu_tx;
cc = node;
cc->status = 0U;
cc->role = 0U;

Expand Down
Loading