Module: xenomai-3 Branch: master Commit: 5460e09d0f990799fb2eed67fbb00358860ebcab URL: http://git.xenomai.org/?p=xenomai-3.git;a=commit;h=5460e09d0f990799fb2eed67fbb00358860ebcab
Author: Philippe Gerum <r...@xenomai.org> Date: Sat Aug 29 12:10:30 2015 +0200 drivers/can, utils/can: introduce can_ifreq Doing piggyback on struct ifreq for passing arguments to CAN-specific control requests is unsafe since nothing relates sizeof(ifr.ifr_ifru) to the largest data field we need. Besides, this is prone to type aliasing issues when mapping the CAN members over the union space using casts, which the compiler may complain about loudly. Introduce struct can_ifreq, defining all the data fields currently supported by CAN raw ioctl requests. The driver still accepts struct ifreq from existing applications though, but it is recommended to switch to can_ifreq at the first opportunity. --- include/rtdm/uapi/can.h | 66 ++++++++++++++------ kernel/drivers/can/rtcan_raw_dev.c | 120 ++++++++++++++++-------------------- utils/can/rtcanconfig.c | 34 ++++------ utils/can/rtcanrecv.c | 2 +- utils/can/rtcansend.c | 2 +- 5 files changed, 112 insertions(+), 112 deletions(-) diff --git a/include/rtdm/uapi/can.h b/include/rtdm/uapi/can.h index 2700694..a33003f 100644 --- a/include/rtdm/uapi/can.h +++ b/include/rtdm/uapi/can.h @@ -324,6 +324,26 @@ typedef struct can_frame { uint8_t data[8] __attribute__ ((aligned(8))); } can_frame_t; +/** + * CAN interface request descriptor + * + * Parameter block for submitting CAN control requests. + */ +struct can_ifreq { + union { + char ifrn_name[IFNAMSIZ]; + } ifr_ifrn; + + union { + struct can_bittime bittime; + can_baudrate_t baudrate; + can_ctrlmode_t ctrlmode; + can_mode_t mode; + can_state_t state; + int ifru_ivalue; + } ifr_ifru; +}; + /*! * @anchor RTCAN_TIMESTAMPS @name Timestamp switches * Arguments to pass to @ref RTCAN_RTIOC_TAKE_TIMESTAMP @@ -445,16 +465,22 @@ typedef struct can_frame { /*! * @anchor CANIOCTLs @name IOCTLs * CAN device IOCTLs + * + * @deprecated Passing <TT>struct ifreq<TT> as a request descriptor + * for CAN IOCTLs is still accepted for backward compatibility, + * however it is recommended to switch to <TT>struct can_ifreq<TT> at + * the first opportunity. + * * @{ */ /** * Get CAN interface index by name * * @param [in,out] arg Pointer to interface request structure buffer - * (<TT>struct ifreq</TT> from linux/if.h). If - * <TT>ifr_name</TT> holds a valid CAN interface name - * <TT>ifr_ifindex</TT> will be filled with the - * corresponding interface index. + * (<TT>struct can_ifreq</TT>). If + * <TT>ifr_name</TT> holds a valid CAN interface + * name <TT>ifr_ifindex</TT> will be filled with + * the corresponding interface index. * * @return 0 on success, otherwise: * - -EFAULT: It was not possible to access user space memory area at the @@ -475,7 +501,7 @@ typedef struct can_frame { * @ref SIOCSCANCUSTOMBITTIME to set custom bit-timing. * * @param [in] arg Pointer to interface request structure buffer - * (<TT>struct ifreq</TT> from linux/if.h). + * (<TT>struct can_ifreq</TT>). * <TT>ifr_name</TT> must hold a valid CAN interface name, * <TT>ifr_ifru</TT> must be filled with an instance of * @ref can_baudrate_t. @@ -493,13 +519,13 @@ typedef struct can_frame { * @note Setting the baud rate is a configuration task. It should * be done deliberately or otherwise CAN messages will likely be lost. */ -#define SIOCSCANBAUDRATE _IOW(RTIOC_TYPE_CAN, 0x01, struct ifreq) +#define SIOCSCANBAUDRATE _IOW(RTIOC_TYPE_CAN, 0x01, struct can_ifreq) /** * Get baud rate * * @param [in,out] arg Pointer to interface request structure buffer - * (<TT>struct ifreq</TT> from linux/if.h). + * (<TT>struct can_ifreq</TT>). * <TT>ifr_name</TT> must hold a valid CAN interface name, * <TT>ifr_ifru</TT> will be filled with an instance of * @ref can_baudrate_t. @@ -512,7 +538,7 @@ typedef struct can_frame { * * @coretags{task-unrestricted} */ -#define SIOCGCANBAUDRATE _IOWR(RTIOC_TYPE_CAN, 0x02, struct ifreq) +#define SIOCGCANBAUDRATE _IOWR(RTIOC_TYPE_CAN, 0x02, struct can_ifreq) /** * Set custom bit time parameter @@ -521,7 +547,7 @@ typedef struct can_frame { * struct can_bittime). * * @param [in] arg Pointer to interface request structure buffer - * (<TT>struct ifreq</TT> from linux/if.h). + * (<TT>struct can_ifreq</TT>). * <TT>ifr_name</TT> must hold a valid CAN interface name, * <TT>ifr_ifru</TT> must be filled with an instance of * struct can_bittime. @@ -538,13 +564,13 @@ typedef struct can_frame { * @note Setting the bit-time is a configuration task. It should * be done deliberately or otherwise CAN messages will likely be lost. */ -#define SIOCSCANCUSTOMBITTIME _IOW(RTIOC_TYPE_CAN, 0x03, struct ifreq) +#define SIOCSCANCUSTOMBITTIME _IOW(RTIOC_TYPE_CAN, 0x03, struct can_ifreq) /** * Get custom bit-time parameters * * @param [in,out] arg Pointer to interface request structure buffer - * (<TT>struct ifreq</TT> from linux/if.h). + * (<TT>struct can_ifreq</TT>). * <TT>ifr_name</TT> must hold a valid CAN interface name, * <TT>ifr_ifru</TT> will be filled with an instance of * struct can_bittime. @@ -557,7 +583,7 @@ typedef struct can_frame { * * @coretags{task-unrestricted} */ -#define SIOCGCANCUSTOMBITTIME _IOWR(RTIOC_TYPE_CAN, 0x04, struct ifreq) +#define SIOCGCANCUSTOMBITTIME _IOWR(RTIOC_TYPE_CAN, 0x04, struct can_ifreq) /** * Set operation mode of CAN controller @@ -565,7 +591,7 @@ typedef struct can_frame { * See @ref CAN_MODE "CAN controller modes" for available modes. * * @param [in] arg Pointer to interface request structure buffer - * (<TT>struct ifreq</TT> from linux/if.h). + * (<TT>struct can_ifreq</TT>). * <TT>ifr_name</TT> must hold a valid CAN interface name, * <TT>ifr_ifru</TT> must be filled with an instance of * @ref can_mode_t. @@ -592,7 +618,7 @@ typedef struct can_frame { * If a controller is bus-off, setting it into stop mode will return no error * but the controller remains bus-off. */ -#define SIOCSCANMODE _IOW(RTIOC_TYPE_CAN, 0x05, struct ifreq) +#define SIOCSCANMODE _IOW(RTIOC_TYPE_CAN, 0x05, struct can_ifreq) /** * Get current state of CAN controller @@ -606,7 +632,7 @@ typedef struct can_frame { * @ref CAN_STATE "CAN controller states". * * @param [in,out] arg Pointer to interface request structure buffer - * (<TT>struct ifreq</TT> from linux/if.h). + * (<TT>struct can_ifreq</TT>). * <TT>ifr_name</TT> must hold a valid CAN interface name, * <TT>ifr_ifru</TT> will be filled with an instance of * @ref can_mode_t. @@ -618,7 +644,7 @@ typedef struct can_frame { * * @coretags{task-unrestricted, might-switch} */ -#define SIOCGCANSTATE _IOWR(RTIOC_TYPE_CAN, 0x06, struct ifreq) +#define SIOCGCANSTATE _IOWR(RTIOC_TYPE_CAN, 0x06, struct can_ifreq) /** * Set special controller modes @@ -627,7 +653,7 @@ typedef struct can_frame { * @ref CAN_CTRLMODE for further information). * * @param [in] arg Pointer to interface request structure buffer - * (<TT>struct ifreq</TT> from linux/if.h). + * (<TT>struct can_ifreq</TT>). * <TT>ifr_name</TT> must hold a valid CAN interface name, * <TT>ifr_ifru</TT> must be filled with an instance of * @ref can_ctrlmode_t. @@ -644,14 +670,14 @@ typedef struct can_frame { * @note Setting special controller modes is a configuration task. It should * be done deliberately or otherwise CAN messages will likely be lost. */ -#define SIOCSCANCTRLMODE _IOW(RTIOC_TYPE_CAN, 0x07, struct ifreq) +#define SIOCSCANCTRLMODE _IOW(RTIOC_TYPE_CAN, 0x07, struct can_ifreq) /** * Get special controller modes * * * @param [in] arg Pointer to interface request structure buffer - * (<TT>struct ifreq</TT> from linux/if.h). + * (<TT>struct can_ifreq</TT>). * <TT>ifr_name</TT> must hold a valid CAN interface name, * <TT>ifr_ifru</TT> must be filled with an instance of * @ref can_ctrlmode_t. @@ -664,7 +690,7 @@ typedef struct can_frame { * * @coretags{task-unrestricted, might-switch} */ -#define SIOCGCANCTRLMODE _IOWR(RTIOC_TYPE_CAN, 0x08, struct ifreq) +#define SIOCGCANCTRLMODE _IOWR(RTIOC_TYPE_CAN, 0x08, struct can_ifreq) /** * Enable or disable storing a high precision timestamp upon reception of diff --git a/kernel/drivers/can/rtcan_raw_dev.c b/kernel/drivers/can/rtcan_raw_dev.c index 999b593..1da6261 100644 --- a/kernel/drivers/can/rtcan_raw_dev.c +++ b/kernel/drivers/can/rtcan_raw_dev.c @@ -233,66 +233,52 @@ static int rtcan_calc_bit_time(struct rtcan_device *dev, #endif /* CONFIG_XENO_DRIVERS_CAN_CALC_BITTIME_OLD */ static inline int rtcan_raw_ioctl_dev_get(struct rtcan_device *dev, - int request, struct ifreq *ifr) + int request, struct can_ifreq *ifr) { - struct can_bittime *bittime; - can_baudrate_t *baudrate = NULL; - can_state_t *state; - can_ctrlmode_t *ctrlmode; rtdm_lockctx_t lock_ctx; - int ret = 0; switch (request) { - case SIOCGIFINDEX: { + case SIOCGIFINDEX: ifr->ifr_ifindex = dev->ifindex; break; - } case SIOCGCANSTATE: - state = (can_state_t *)&ifr->ifr_ifru; rtdm_lock_get_irqsave(&dev->device_lock, lock_ctx); if (dev->do_get_state) dev->state = dev->do_get_state(dev); - *state = dev->state; + ifr->ifr_ifru.state = dev->state; rtdm_lock_put_irqrestore(&dev->device_lock, lock_ctx); break; case SIOCGCANCTRLMODE: - ctrlmode = (can_ctrlmode_t *)&ifr->ifr_ifru; - *ctrlmode = dev->ctrl_mode; + ifr->ifr_ifru.ctrlmode = dev->ctrl_mode; break; case SIOCGCANBAUDRATE: - baudrate = (can_baudrate_t *)&ifr->ifr_ifru; - *baudrate = dev->baudrate; + ifr->ifr_ifru.baudrate = dev->baudrate; break; case SIOCGCANCUSTOMBITTIME: - bittime = (struct can_bittime *)&ifr->ifr_ifru; - *bittime = dev->bit_time; + ifr->ifr_ifru.bittime = dev->bit_time; break; } - return ret; + return 0; } static inline int rtcan_raw_ioctl_dev_set(struct rtcan_device *dev, - int request, struct ifreq *ifr) + int request, struct can_ifreq *ifr) { rtdm_lockctx_t lock_ctx; - can_ctrlmode_t *ctrl_mode; - can_mode_t *mode; int ret = 0, started = 0; struct can_bittime bit_time, *bt; - can_baudrate_t *baudrate = NULL; switch (request) { case SIOCSCANBAUDRATE: if (!dev->do_set_bit_time) return 0; - baudrate = (can_baudrate_t *)&ifr->ifr_ifru; - ret = rtcan_calc_bit_time(dev, *baudrate, &bit_time.std); + ret = rtcan_calc_bit_time(dev, ifr->ifr_ifru.baudrate, &bit_time.std); if (ret) break; bit_time.type = CAN_BITTIME_STD; @@ -317,27 +303,26 @@ static inline int rtcan_raw_ioctl_dev_set(struct rtcan_device *dev, switch (request) { case SIOCSCANMODE: - mode = (can_mode_t *)&ifr->ifr_ifru; if (dev->do_set_mode && - !(*mode == CAN_MODE_START && CAN_STATE_OPERATING(dev->state))) - ret = dev->do_set_mode(dev, *mode, &lock_ctx); + !(ifr->ifr_ifru.mode == CAN_MODE_START && + CAN_STATE_OPERATING(dev->state))) + ret = dev->do_set_mode(dev, ifr->ifr_ifru.mode, &lock_ctx); break; case SIOCSCANCTRLMODE: - ctrl_mode = (can_ctrlmode_t *)&ifr->ifr_ifru; - dev->ctrl_mode = *ctrl_mode; + dev->ctrl_mode = ifr->ifr_ifru.ctrlmode; break; case SIOCSCANBAUDRATE: ret = dev->do_set_bit_time(dev, &bit_time, &lock_ctx); if (!ret) { - dev->baudrate = *baudrate; + dev->baudrate = ifr->ifr_ifru.baudrate; dev->bit_time = bit_time; } break; case SIOCSCANCUSTOMBITTIME: - bt = (struct can_bittime *)&ifr->ifr_ifru; + bt = &ifr->ifr_ifru.bittime; ret = dev->do_set_bit_time(dev, bt, &lock_ctx); if (!ret) { dev->bit_time = *bt; @@ -354,7 +339,6 @@ static inline int rtcan_raw_ioctl_dev_set(struct rtcan_device *dev, default: ret = -EOPNOTSUPP; break; - } out: @@ -368,9 +352,18 @@ static inline int rtcan_raw_ioctl_dev_set(struct rtcan_device *dev, int rtcan_raw_ioctl_dev(struct rtdm_fd *fd, int request, void *arg) { - int ret = 0; - struct ifreq *ifr; - struct ifreq ifr_buf; + struct can_ifreq *ifr; + int ret = 0, get = 0; + union { + /* + * We need to deal with callers still passing struct ifreq + * instead of can_ifreq, which might have a larger memory + * footprint (but can't be smaller though). Field offsets + * will be the same regardless. + */ + struct ifreq ifr_legacy; + struct can_ifreq ifr_can; + } ifr_buf; struct rtcan_device *dev; switch (request) { @@ -379,53 +372,46 @@ int rtcan_raw_ioctl_dev(struct rtdm_fd *fd, int request, void *arg) case SIOCGCANSTATE: case SIOCGCANBAUDRATE: case SIOCGCANCUSTOMBITTIME: - - if (rtdm_fd_is_user(fd)) { - if (!rtdm_rw_user_ok(fd, arg, sizeof(struct ifreq)) || - rtdm_copy_from_user(fd, &ifr_buf, arg, - sizeof(struct ifreq))) - return -EFAULT; - - ifr = &ifr_buf; - } else - ifr = (struct ifreq *)arg; - - if ((dev = rtcan_dev_get_by_name(ifr->ifr_name)) == NULL) - return -ENODEV; - ret = rtcan_raw_ioctl_dev_get(dev, request, ifr); - rtcan_dev_dereference(dev); - - if (rtdm_fd_is_user(fd) && !ret) { - /* Since we yet tested if user memory is rw safe, - we can copy to user space directly */ - if (rtdm_copy_to_user(fd, arg, ifr, - sizeof(struct ifreq))) - return -EFAULT; - } - break; - + get = 1; + /* Falldown wanted. */ case SIOCSCANMODE: case SIOCSCANCTRLMODE: case SIOCSCANBAUDRATE: case SIOCSCANCUSTOMBITTIME: if (rtdm_fd_is_user(fd)) { - /* Copy struct ifreq from userspace */ + /* Copy struct can_ifreq from userspace */ if (!rtdm_read_user_ok(fd, arg, - sizeof(struct ifreq)) || + sizeof(struct can_ifreq)) || rtdm_copy_from_user(fd, &ifr_buf, arg, - sizeof(struct ifreq))) + sizeof(struct can_ifreq))) return -EFAULT; - ifr = &ifr_buf; + ifr = &ifr_buf.ifr_can; } else - ifr = (struct ifreq *)arg; + ifr = (struct can_ifreq *)arg; /* Get interface index and data */ - if ((dev = rtcan_dev_get_by_name(ifr->ifr_name)) == NULL) + dev = rtcan_dev_get_by_name(ifr->ifr_name); + if (dev == NULL) return -ENODEV; - ret = rtcan_raw_ioctl_dev_set(dev, request, ifr); - rtcan_dev_dereference(dev); + + if (get) { + ret = rtcan_raw_ioctl_dev_get(dev, request, ifr); + rtcan_dev_dereference(dev); + if (ret == 0 && rtdm_fd_is_user(fd)) { + /* + * Since we yet tested if user memory is rw safe, + * we can copy to user space directly. + */ + if (rtdm_copy_to_user(fd, arg, ifr, + sizeof(struct can_ifreq))) + return -EFAULT; + } + } else { + ret = rtcan_raw_ioctl_dev_set(dev, request, ifr); + rtcan_dev_dereference(dev); + } break; default: diff --git a/utils/can/rtcanconfig.c b/utils/can/rtcanconfig.c index dcf551b..968e909 100644 --- a/utils/can/rtcanconfig.c +++ b/utils/can/rtcanconfig.c @@ -88,16 +88,7 @@ int main(int argc, char *argv[]) int new_ctrlmode = 0, set_ctrlmode = 0; int verbose = 0; int bittime_count = 0, bittime_data[6]; - can_baudrate_t *baudrate; - can_ctrlmode_t *ctrlmode; - can_mode_t *mode; - union { - struct ifreq ifr; - struct can_bittime bittime; - can_baudrate_t baudrate; - can_ctrlmode_t ctrlmode; - can_mode_t mode; - } u; + struct can_ifreq ifr; struct can_bittime *bittime; int opt, ret; char* ptr; @@ -169,7 +160,7 @@ int main(int argc, char *argv[]) } strncpy(ifname, argv[optind], IFNAMSIZ); - strncpy(u.ifr.ifr_name, ifname, IFNAMSIZ); + strncpy(ifr.ifr_name, ifname, IFNAMSIZ); if (optind == argc - 2) { /* Get mode setting */ new_mode = string_to_mode(argv[optind + 1]); @@ -187,7 +178,7 @@ int main(int argc, char *argv[]) return can_fd; } - ret = ioctl(can_fd, SIOCGIFINDEX, &u.ifr); + ret = ioctl(can_fd, SIOCGIFINDEX, &ifr); if (ret) { fprintf(stderr,"Can't get interface index for %s, code = %d\n", ifname, ret); return ret; @@ -197,16 +188,15 @@ int main(int argc, char *argv[]) if (new_baudrate != -1) { if (verbose) printf("baudrate: %d\n", new_baudrate); - baudrate = &u.baudrate; - *baudrate = new_baudrate; - ret = ioctl(can_fd, SIOCSCANBAUDRATE, &u.ifr); + ifr.ifr_ifru.baudrate = new_baudrate; + ret = ioctl(can_fd, SIOCSCANBAUDRATE, &ifr); if (ret) { goto abort; } } if (bittime_count) { - bittime = &u.bittime; + bittime = &ifr.ifr_ifru.bittime; if (bittime_count == 2) { bittime->type = CAN_BITTIME_BTR; bittime->btr.btr0 = bittime_data[0]; @@ -233,7 +223,7 @@ int main(int argc, char *argv[]) bittime->std.sam); } - ret = ioctl(can_fd, SIOCSCANCUSTOMBITTIME, &u.ifr); + ret = ioctl(can_fd, SIOCSCANCUSTOMBITTIME, &ifr); if (ret) { goto abort; } @@ -241,20 +231,18 @@ int main(int argc, char *argv[]) } if (set_ctrlmode != 0) { - ctrlmode = &u.ctrlmode; - *ctrlmode = new_ctrlmode; + ifr.ifr_ifru.ctrlmode = new_ctrlmode; if (verbose) printf("ctrlmode: %#x\n", new_ctrlmode); - ret = ioctl(can_fd, SIOCSCANCTRLMODE, &u.ifr); + ret = ioctl(can_fd, SIOCSCANCTRLMODE, &ifr); if (ret) { goto abort; } } if (new_mode != -1) { - mode = &u.mode; - *mode = new_mode; - ret = ioctl(can_fd, SIOCSCANMODE, &u.ifr); + ifr.ifr_ifru.mode = new_mode; + ret = ioctl(can_fd, SIOCSCANMODE, &ifr); if (ret) { goto abort; } diff --git a/utils/can/rtcanrecv.c b/utils/can/rtcanrecv.c index e6e87b3..8155ab7 100644 --- a/utils/can/rtcanrecv.c +++ b/utils/can/rtcanrecv.c @@ -160,7 +160,7 @@ int main(int argc, char **argv) int opt, ret; u_int32_t id, mask; u_int32_t err_mask = 0; - struct ifreq ifr; + struct can_ifreq ifr; char *ptr; char name[32]; diff --git a/utils/can/rtcansend.c b/utils/can/rtcansend.c index 51fc080..4a692b3 100644 --- a/utils/can/rtcansend.c +++ b/utils/can/rtcansend.c @@ -120,7 +120,7 @@ static void rt_task(void) int main(int argc, char **argv) { int i, opt, ret; - struct ifreq ifr; + struct can_ifreq ifr; char name[32]; struct option long_options[] = { _______________________________________________ Xenomai-git mailing list Xenomai-git@xenomai.org http://xenomai.org/mailman/listinfo/xenomai-git