Hi Luiz,
The unaligned error is raised when trying to access the address of mtu
of a packed struct where mtu's size is 2 bytes; which means accessing
the address of a packed member whose size is greater than 1 byte can
raised this error. So it is not true that there is no error as far as
the member is not accessed via void *(see the following function
signature).
static inline bool l2cap_frame_get_le16(struct l2cap_frame *frame,
uint16_t *value)
Here are the errors generated. The flags are "Werror" and
"Waddress-of-packed-member". I will also add these two errors into the
commit message.
monitor/rfcomm.c:238:36: error: taking address of packed member 'pm'
of class or structure 'rfcomm_rpn' may result in an unaligned pointer
value [-Werror,-Waddress-of-packed-member]
if (!l2cap_frame_get_le16(frame, &rpn.pm))
monitor/rfcomm.c:287:36: error: taking address of packed member 'mtu'
of class or structure 'rfcomm_pn' may result in an unaligned pointer
value [-Werror,-Waddress-of-packed-member]
if (!l2cap_frame_get_le16(frame, &pn.mtu))
Thanks,
Miao
On Wed, Dec 21, 2016 at 6:03 AM, Luiz Augusto von Dentz
<luiz.dentz@xxxxxxxxx> wrote:
> Hi,
>
> On Tue, Dec 20, 2016 at 9:50 PM, <mcchou@xxxxxxxxxxxx> wrote:
>> From: Miao-chen Chou <mcchou@xxxxxxxxxxxx>
>>
>> This patch removes "packed" attribute from the definition of struct rfcomm_rpn
>> to prevent the access to an unaligned struct member in mmc_rpn(). This patch
>> also introduces a temp variable in mcc_pn() to prevent unaligned access without
>> touching the definition of struct rfcomm_pn, since struct rfcomm_pn is used as
>> a PDU.
>> ---
>> v4 removes the unrelated change on the code format.
>>
>> monitor/rfcomm.c | 7 +++++--
>> 1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/monitor/rfcomm.c b/monitor/rfcomm.c
>> index b32ad40..4bd549c 100644
>> --- a/monitor/rfcomm.c
>> +++ b/monitor/rfcomm.c
>> @@ -106,7 +106,7 @@ struct rfcomm_rpn {
>> uint8_t xon;
>> uint8_t xoff;
>> uint16_t pm;
>> -} __attribute__ ((packed));
>> +};
>>
>> struct rfcomm_rls {
>> uint8_t dlci;
>> @@ -265,6 +265,7 @@ static inline bool mcc_pn(struct rfcomm_frame *rfcomm_frame, uint8_t indent)
>> {
>> struct l2cap_frame *frame = &rfcomm_frame->l2cap_frame;
>> struct rfcomm_pn pn;
>> + uint16_t mtu;
>>
>> /* rfcomm_pn struct is defined in rfcomm.h */
>>
>> @@ -284,8 +285,10 @@ static inline bool mcc_pn(struct rfcomm_frame *rfcomm_frame, uint8_t indent)
>> if (!l2cap_frame_get_u8(frame, &pn.ack_timer))
>> return false;
>>
>> - if (!l2cap_frame_get_le16(frame, &pn.mtu))
>> + /* prevent unaligned memory access */
>> + if (!l2cap_frame_get_le16(frame, &mtu))
>> return false;
>> + pn.mtu = mtu;
>
> Interesting, isn't this causing unaligned access anyway since it is
> assigning to pn.mtu which is packed? Or this is fine as far as it is
> not accessed via void *? Btw, please add the compiler output and flags
> it is being used, it is quite possible we can replicate the same
> errors on gcc or perhaps it is missing some flags used in gcc to make
> this aligned?
>
>> if (!l2cap_frame_get_u8(frame, &pn.max_retrans))
>> return false;
>> --
>> 2.8.0.rc3.226.g39d4020
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
>> the body of a message to majordomo@xxxxxxxxxxxxxxx
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
>
> --
> Luiz Augusto von Dentz
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
