Re: [Xenomai-core] [PATCH] posix: Fix error checks when copying user strings
Jan Kiszka wrote: > Do not return -EFAULT when the passed string has zero-length. Instead, > return -EINVAL when trying to create objects with empty names. Ok. You can commit this. -- Gilles. ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] [PATCH] posix: Fix error checks when copying user strings
Jan Kiszka wrote: > Jan Kiszka wrote: >> Jan Kiszka wrote: >>> Gilles Chanteperdrix wrote: Jan Kiszka wrote: > Gilles Chanteperdrix wrote: >> Jan Kiszka wrote: >>> Gilles Chanteperdrix wrote: Jan Kiszka wrote: > Obviously a conversion error while switching to __xn_safe*. > > Signed-off-by: Jan Kiszka Well, I have just checked the kernel code, and 0 as a return value of strncpy_from_user is treated as a value in most places, even if not -EFAULT. >>> Better check our code: :) __xn_safe_strncpy_from_user works differently. >> Then I would tend to consider that xn_safe_strncpy is broken. > No, because it not a derivate of strncpy_from_user, but an internal > service optimized for the most common use cases (where you don't care > about the precise return value). So, what should I call if I care about the return value ? >>> The old combo of access_rok() and __xn_strncpy_from_user() - ah, I see >>> the issue: POSIX requires the length to report overflows to the users. >>> Hmm, then back to the old code, just adding the missing address range check. >>> >> Forget that, now I should have looked into our code: The "safe" version >> transparently calls __xn_strncpy_from_user, no need to go by foot here. >> > > Here is the real bug: > >> long res; >> __do_strncpy_from_user(dst, src, count, res); >> 804505cc: 48 85 c9test %rcx,%rcx >> 804505cf: 74 0e je 804505df >> >> 804505d1: ac lods %ds:(%rsi),%al >> 804505d2: aa stos %al,%es:(%rdi) >> 804505d3: 84 c0 test %al,%al >> 804505d5: 74 05 je 804505dc >> >> 804505d7: 48 ff c9dec%rcx >> 804505da: 75 f5 jne804505d1 >> >> 804505dc: 48 29 c9sub%rcx,%rcx > ^ > >> return res; >> } >> 804505df: 48 89 c8mov%rcx,%rax >> 804505e2: c9 leaveq >> 804505e3: c3 retq > > And that is due to us lacking kernel commit > e0a96129db574d6365e3439d16d88517c437ab33. > > Nevertheless, the existing checks against 0 in the POSIX layer weren't > correct either. We need to fix both, but less ad-hoc than I tried. IMO, sem_open(""), mq_open(""), shm_open("") should return an error. I agree that -EFAULT is a bit harsh and that -EINVAL would be more apropriate, but I do not want to remove that check. -- Gilles. ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] [PATCH] posix: Fix error checks when copying user strings
Jan Kiszka wrote: > Jan Kiszka wrote: >> Gilles Chanteperdrix wrote: >>> Jan Kiszka wrote: Gilles Chanteperdrix wrote: > Jan Kiszka wrote: >> Gilles Chanteperdrix wrote: >>> Jan Kiszka wrote: Obviously a conversion error while switching to __xn_safe*. Signed-off-by: Jan Kiszka >>> Well, I have just checked the kernel code, and 0 as a return value of >>> strncpy_from_user is treated as a value in most places, even if not >>> -EFAULT. >>> >> Better check our code: :) __xn_safe_strncpy_from_user works differently. > Then I would tend to consider that xn_safe_strncpy is broken. No, because it not a derivate of strncpy_from_user, but an internal service optimized for the most common use cases (where you don't care about the precise return value). >>> So, what should I call if I care about the return value ? >> The old combo of access_rok() and __xn_strncpy_from_user() - ah, I see >> the issue: POSIX requires the length to report overflows to the users. >> Hmm, then back to the old code, just adding the missing address range check. >> > > Forget that, now I should have looked into our code: The "safe" version > transparently calls __xn_strncpy_from_user, no need to go by foot here. > Here is the real bug: > long res; > __do_strncpy_from_user(dst, src, count, res); > 804505cc: 48 85 c9test %rcx,%rcx > 804505cf: 74 0e je 804505df > > 804505d1: ac lods %ds:(%rsi),%al > 804505d2: aa stos %al,%es:(%rdi) > 804505d3: 84 c0 test %al,%al > 804505d5: 74 05 je 804505dc > > 804505d7: 48 ff c9dec%rcx > 804505da: 75 f5 jne804505d1 > > 804505dc: 48 29 c9sub%rcx,%rcx ^ > return res; > } > 804505df: 48 89 c8mov%rcx,%rax > 804505e2: c9 leaveq > 804505e3: c3 retq And that is due to us lacking kernel commit e0a96129db574d6365e3439d16d88517c437ab33. Nevertheless, the existing checks against 0 in the POSIX layer weren't correct either. We need to fix both, but less ad-hoc than I tried. Jan signature.asc Description: OpenPGP digital signature ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] [PATCH] posix: Fix error checks when copying user strings
Jan Kiszka wrote: > Gilles Chanteperdrix wrote: >> Jan Kiszka wrote: >>> Gilles Chanteperdrix wrote: Jan Kiszka wrote: > Gilles Chanteperdrix wrote: >> Jan Kiszka wrote: >>> Obviously a conversion error while switching to __xn_safe*. >>> >>> Signed-off-by: Jan Kiszka >> Well, I have just checked the kernel code, and 0 as a return value of >> strncpy_from_user is treated as a value in most places, even if not >> -EFAULT. >> > Better check our code: :) __xn_safe_strncpy_from_user works differently. Then I would tend to consider that xn_safe_strncpy is broken. >>> No, because it not a derivate of strncpy_from_user, but an internal >>> service optimized for the most common use cases (where you don't care >>> about the precise return value). >> So, what should I call if I care about the return value ? > > The old combo of access_rok() and __xn_strncpy_from_user() - ah, I see > the issue: POSIX requires the length to report overflows to the users. > Hmm, then back to the old code, just adding the missing address range check. > Forget that, now I should have looked into our code: The "safe" version transparently calls __xn_strncpy_from_user, no need to go by foot here. Jan -- Siemens AG, Corporate Technology, CT SE 2 Corporate Competence Center Embedded Linux ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] [PATCH] posix: Fix error checks when copying user strings
Gilles Chanteperdrix wrote: > Jan Kiszka wrote: >> Gilles Chanteperdrix wrote: >>> Jan Kiszka wrote: Gilles Chanteperdrix wrote: > Jan Kiszka wrote: >> Obviously a conversion error while switching to __xn_safe*. >> >> Signed-off-by: Jan Kiszka > Well, I have just checked the kernel code, and 0 as a return value of > strncpy_from_user is treated as a value in most places, even if not > -EFAULT. > Better check our code: :) __xn_safe_strncpy_from_user works differently. >>> Then I would tend to consider that xn_safe_strncpy is broken. >> No, because it not a derivate of strncpy_from_user, but an internal >> service optimized for the most common use cases (where you don't care >> about the precise return value). > > So, what should I call if I care about the return value ? The old combo of access_rok() and __xn_strncpy_from_user() - ah, I see the issue: POSIX requires the length to report overflows to the users. Hmm, then back to the old code, just adding the missing address range check. Jan -- Siemens AG, Corporate Technology, CT SE 2 Corporate Competence Center Embedded Linux ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] [PATCH] posix: Fix error checks when copying user strings
Jan Kiszka wrote: > Gilles Chanteperdrix wrote: >> Jan Kiszka wrote: >>> Gilles Chanteperdrix wrote: Jan Kiszka wrote: > Obviously a conversion error while switching to __xn_safe*. > > Signed-off-by: Jan Kiszka Well, I have just checked the kernel code, and 0 as a return value of strncpy_from_user is treated as a value in most places, even if not -EFAULT. >>> Better check our code: :) __xn_safe_strncpy_from_user works differently. >> Then I would tend to consider that xn_safe_strncpy is broken. > > No, because it not a derivate of strncpy_from_user, but an internal > service optimized for the most common use cases (where you don't care > about the precise return value). So, what should I call if I care about the return value ? -- Gilles. ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] [PATCH] posix: Fix error checks when copying user strings
Gilles Chanteperdrix wrote: > Jan Kiszka wrote: >> Gilles Chanteperdrix wrote: >>> Jan Kiszka wrote: Obviously a conversion error while switching to __xn_safe*. Signed-off-by: Jan Kiszka >>> Well, I have just checked the kernel code, and 0 as a return value of >>> strncpy_from_user is treated as a value in most places, even if not -EFAULT. >>> >> Better check our code: :) __xn_safe_strncpy_from_user works differently. > > Then I would tend to consider that xn_safe_strncpy is broken. No, because it not a derivate of strncpy_from_user, but an internal service optimized for the most common use cases (where you don't care about the precise return value). Jan -- Siemens AG, Corporate Technology, CT SE 2 Corporate Competence Center Embedded Linux ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] [PATCH] posix: Fix error checks when copying user strings
Jan Kiszka wrote: > Gilles Chanteperdrix wrote: >> Jan Kiszka wrote: >>> Obviously a conversion error while switching to __xn_safe*. >>> >>> Signed-off-by: Jan Kiszka >> Well, I have just checked the kernel code, and 0 as a return value of >> strncpy_from_user is treated as a value in most places, even if not -EFAULT. >> > > Better check our code: :) __xn_safe_strncpy_from_user works differently. Then I would tend to consider that xn_safe_strncpy is broken. -- Gilles. ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] [PATCH] posix: Fix error checks when copying user strings
Gilles Chanteperdrix wrote: > Jan Kiszka wrote: >> Obviously a conversion error while switching to __xn_safe*. >> >> Signed-off-by: Jan Kiszka > > Well, I have just checked the kernel code, and 0 as a return value of > strncpy_from_user is treated as a value in most places, even if not -EFAULT. > Better check our code: :) __xn_safe_strncpy_from_user works differently. Jan -- Siemens AG, Corporate Technology, CT SE 2 Corporate Competence Center Embedded Linux ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] [PATCH] posix: Fix error checks when copying user strings
Jan Kiszka wrote: > Obviously a conversion error while switching to __xn_safe*. > > Signed-off-by: Jan Kiszka Well, I have just checked the kernel code, and 0 as a return value of strncpy_from_user is treated as a value in most places, even if not -EFAULT. -- Gilles. ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core