Re: [RFC] pps: fixing CONFIG_COMPAT issues

[Matt Ranostay]

On Fri, Dec 23, 2016 at 7:04 AM, Rodolfo Giometti  wrote:
> On 12/22/16 21:39, Matt Ranostay wrote:
>>
>>
>> What would be the best way to fix the padding issue without breaking
>> userspace applications? Just fixing the alignment with explicit
>> padding is of course the clean easy way, but bashing the data in
>> compat_ioctl would avoid breakage.
>
>
> Hi Matt,
>
> I've no experience in this topic... I'm sorry! :(
>
> Maybe is better waiting for David's advices? In the meantime I'm going to
> study the problem a bit better.

Just poking to see if anyone has a better solution. Since the long
holidays are over :)

Thanks,

Matt

>
> Ciao,
>
> Rodolfo
>
> --
>
> HCE Engineering  e-mail: giome...@hce-engineering.com
> GNU/Linux Solutions  giome...@enneenne.com
> Linux Device Driver  giome...@linux.it
> Embedded Systems phone:  +39 349 2432127
> UNIX programming skype:  rodolfo.giometti
> Cosino Project - the quick prototyping embedded system - www.cosino.io
> Freelance ICT Italia - Consulente ICT Italia - www.consulenti-ict.it

Re: [RFC] pps: fixing CONFIG_COMPAT issues

[Matt Ranostay]

On Fri, Dec 23, 2016 at 7:04 AM, Rodolfo Giometti  wrote:
> On 12/22/16 21:39, Matt Ranostay wrote:
>>
>>
>> What would be the best way to fix the padding issue without breaking
>> userspace applications? Just fixing the alignment with explicit
>> padding is of course the clean easy way, but bashing the data in
>> compat_ioctl would avoid breakage.
>
>
> Hi Matt,
>
> I've no experience in this topic... I'm sorry! :(
>
> Maybe is better waiting for David's advices? In the meantime I'm going to
> study the problem a bit better.

Just poking to see if anyone has a better solution. Since the long
holidays are over :)

Thanks,

Matt

>
> Ciao,
>
> Rodolfo
>
> --
>
> HCE Engineering  e-mail: giome...@hce-engineering.com
> GNU/Linux Solutions  giome...@enneenne.com
> Linux Device Driver  giome...@linux.it
> Embedded Systems phone:  +39 349 2432127
> UNIX programming skype:  rodolfo.giometti
> Cosino Project - the quick prototyping embedded system - www.cosino.io
> Freelance ICT Italia - Consulente ICT Italia - www.consulenti-ict.it

Re: [RFC] pps: fixing CONFIG_COMPAT issues

[Rodolfo Giometti]

On 12/22/16 21:39, Matt Ranostay wrote:


What would be the best way to fix the padding issue without breaking
userspace applications? Just fixing the alignment with explicit
padding is of course the clean easy way, but bashing the data in
compat_ioctl would avoid breakage.


Hi Matt,

I've no experience in this topic... I'm sorry! :(

Maybe is better waiting for David's advices? In the meantime I'm going to study 
the problem a bit better.


Ciao,

Rodolfo

--

HCE Engineering  e-mail: giome...@hce-engineering.com
GNU/Linux Solutions  giome...@enneenne.com
Linux Device Driver  giome...@linux.it
Embedded Systems phone:  +39 349 2432127
UNIX programming skype:  rodolfo.giometti
Cosino Project - the quick prototyping embedded system - www.cosino.io
Freelance ICT Italia - Consulente ICT Italia - www.consulenti-ict.it

Re: [RFC] pps: fixing CONFIG_COMPAT issues

[Rodolfo Giometti]

On 12/22/16 21:39, Matt Ranostay wrote:


What would be the best way to fix the padding issue without breaking
userspace applications? Just fixing the alignment with explicit
padding is of course the clean easy way, but bashing the data in
compat_ioctl would avoid breakage.


Hi Matt,

I've no experience in this topic... I'm sorry! :(

Maybe is better waiting for David's advices? In the meantime I'm going to study 
the problem a bit better.


Ciao,

Rodolfo

--

HCE Engineering  e-mail: giome...@hce-engineering.com
GNU/Linux Solutions  giome...@enneenne.com
Linux Device Driver  giome...@linux.it
Embedded Systems phone:  +39 349 2432127
UNIX programming skype:  rodolfo.giometti
Cosino Project - the quick prototyping embedded system - www.cosino.io
Freelance ICT Italia - Consulente ICT Italia - www.consulenti-ict.it

[RFC] pps: fixing CONFIG_COMPAT issues

[Matt Ranostay]

Rodolfo,

I'd like to get some feedback on what would be an upstreamable patch
series for correcting some issues with a 64-bit kernel and using a
32-bit userspace.

First issue is the compat_ioctl has to be sort of hacked since the
IOCTL defines are using pointer sizes in the macro generation (which
works if you don't mix bit sizes of the kernel and userspace) which
should have been just the struct size originally

#define PPS_GETPARAMS   _IOR('p', 0xa1, struct pps_kparams *)
#define PPS_SETPARAMS   _IOW('p', 0xa2, struct pps_kparams *)
#define PPS_GETCAP  _IOR('p', 0xa3, int *)
#define PPS_FETCH   _IOWR('p', 0xa4, struct pps_fdata *)
#define PPS_KC_BIND _IOW('p', 0xa5, struct pps_bind_args *)

So basically the workaround we have for that is as follows:

...
static long pps_cdev_compat_ioctl(struct file *file,
   unsigned int cmd, unsigned long arg)
{
   cmd = _IOC(_IOC_DIR(cmd), _IOC_TYPE(cmd), _IOC_NR(cmd), sizeof(void *));
   return pps_cdev_ioctl(file, cmd, arg);
}
...


Now the bigger and more ABI breaking issue is with the padding in
struct pps_ktime (which David Woodhouse has a comment in the header).
Which is worked around by __attribute__((aligned, 4)) which of course
breaks userspace for a precompiled 64-bit binary.

So the issue is that on 32-bit x86 aligns at 4-bytes, but 64-bit is at
8-bytes, so the data ferried from the former userspace to the latter
kernel is incorrect. For instance struct pps_kinfo is padded 4 bytes
more in between info and timeout members.

What would be the best way to fix the padding issue without breaking
userspace applications? Just fixing the alignment with explicit
padding is of course the clean easy way, but bashing the data in
compat_ioctl would avoid breakage.


Thanks,

Matt

[RFC] pps: fixing CONFIG_COMPAT issues

[Matt Ranostay]

Rodolfo,

I'd like to get some feedback on what would be an upstreamable patch
series for correcting some issues with a 64-bit kernel and using a
32-bit userspace.

First issue is the compat_ioctl has to be sort of hacked since the
IOCTL defines are using pointer sizes in the macro generation (which
works if you don't mix bit sizes of the kernel and userspace) which
should have been just the struct size originally

#define PPS_GETPARAMS   _IOR('p', 0xa1, struct pps_kparams *)
#define PPS_SETPARAMS   _IOW('p', 0xa2, struct pps_kparams *)
#define PPS_GETCAP  _IOR('p', 0xa3, int *)
#define PPS_FETCH   _IOWR('p', 0xa4, struct pps_fdata *)
#define PPS_KC_BIND _IOW('p', 0xa5, struct pps_bind_args *)

So basically the workaround we have for that is as follows:

...
static long pps_cdev_compat_ioctl(struct file *file,
   unsigned int cmd, unsigned long arg)
{
   cmd = _IOC(_IOC_DIR(cmd), _IOC_TYPE(cmd), _IOC_NR(cmd), sizeof(void *));
   return pps_cdev_ioctl(file, cmd, arg);
}
...


Now the bigger and more ABI breaking issue is with the padding in
struct pps_ktime (which David Woodhouse has a comment in the header).
Which is worked around by __attribute__((aligned, 4)) which of course
breaks userspace for a precompiled 64-bit binary.

So the issue is that on 32-bit x86 aligns at 4-bytes, but 64-bit is at
8-bytes, so the data ferried from the former userspace to the latter
kernel is incorrect. For instance struct pps_kinfo is padded 4 bytes
more in between info and timeout members.

What would be the best way to fix the padding issue without breaking
userspace applications? Just fixing the alignment with explicit
padding is of course the clean easy way, but bashing the data in
compat_ioctl would avoid breakage.


Thanks,

Matt