Am Di., 12. Mai 2020 um 22:09 Uhr schrieb Laurent Vivier <laur...@vivier.eu>:
>
> Le 06/05/2020 à 15:21, Tomas Krcka a écrit :
> > Signed-off-by: Tomas Krcka <tomas.kr...@gmail.com>
> > ---
> > linux-user/syscall.c | 34 ++++++++++++++++++++++++++++++++++
> > 1 file changed, 34 insertions(+)
> >
> > diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> > index 05f03919ff..88d4c85b70 100644
> > --- a/linux-user/syscall.c
> > +++ b/linux-user/syscall.c
> > @@ -56,6 +56,7 @@
> > #include <linux/wireless.h>
> > #include <linux/icmp.h>
> > #include <linux/icmpv6.h>
> > +#include <linux/can/raw.h>
> > #include <linux/errqueue.h>
> > #include <linux/random.h>
> > #ifdef CONFIG_TIMERFD
> > @@ -2111,6 +2112,39 @@ static abi_long do_setsockopt(int sockfd, int level,
> > int optname,
> > goto unimplemented;
> > }
> > break;
> > + case SOL_CAN_RAW:
> > + switch (optname) {
> > + case CAN_RAW_FILTER:
> > + {
> > + if (optlen % sizeof(struct can_filter) != 0) {
> > + return -TARGET_EINVAL;
> > + }
> > +
> > + struct can_filter *can_filters = NULL;
>
> Move the declaration to the top of the block.
>
> > + if (optlen != 0) {
>
> If you check, like in kernel, "optlen > CAN_RAW_FILTER_MAX *
> sizeof(struct can_filter)", you can exit here (and no need to set
> can_filters to NULL).
>
The optlen can be 0 and then the can_filter shall be NULL, based on
the socketcan
documentation.
And an additional question, shall I check if optlen is 1 and then use
non-dynamic allocated
filters, as it's done in kernel?
> > + can_filters = g_new0(struct can_filter, optlen);
> > +
> > + if (!can_filters) {
>
> no need to check the result, g_new0() aborts in case of problem.
>
> > + return -TARGET_ENOMEM;
> > + }
> > + if (copy_from_user(can_filters, optval_addr, optlen)) {
> > + g_free(can_filters);
> > + return -TARGET_EFAULT;
> > + }
>
> It would be cleaner not to use the copy_from_user() as we need to
> byte-swap all the fields in the loop below (I know it's done like that
> in SOL_ICMPV6...)
>
> > + for (val = 0; val < optlen / sizeof(struct can_filter);
> > val++) {
> > + can_filters[val].can_id =
> > tswap32(can_filters[val].can_id);
> > + can_filters[val].can_mask =
> > tswap32(can_filters[val].can_mask);
> > + }
>
> So, something like:
>
> target_can_filters = lock_user(VERIFY_READ, optval_addr, optlen, 1);
> for (val = 0; val < optlen / sizeof(struct can_filter); val++) {
> __get_user(can_filters[val].can_id, \
> &target_can_filters[val].can_id);
> __get_user(can_filters[val].can_mask, \
> &target_can_filters[val].can_mask);
> }
> unlock_user(target_can_filters);
>
> > + }
> > + ret = get_errno(setsockopt(sockfd, level, optname,
> > + can_filters, optlen));
> > + g_free(can_filters);
> > + break;
> > + }
> > + default:
> > + goto unimplemented;
> > + }
> > + break;
> > case SOL_RAW:
> > switch (optname) {
> > case ICMP_FILTER:
> >
>
> Could you also update getsockopt()?
Yes, I can.
>
> Thanks,
> Laurent
>
>
