This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
Other format: | [Raw text] |
Hi Joseph, > On Mon, 28 Oct 2019, Lukasz Majewski wrote: > > > The new helper function - __utimensat64_helper - has been > > introduced to facilitate code re-usage on function providing > > futimens syscall handling. It also checks if passed nanoseconds are > > in the valid range as well as if one of two special values - > > UTIME_NOW and UTIME_OMIT were not passed. > > In general we don't need such a check in userspace if the nanoseconds > value will be passed to the kernel and it will do its own check. I > don't see such a check in the original code. I must admit that I was a bit too cautious. If I remember correctly kernel does the valid nano seconds check on its own, so it would be correct to allow it to perform the check and return the error if wrong range is detected. > Is it being added to > give the EINVAL error precedence over EOVERFLOW, in the case where a > 64-bit time value with invalid nanoseconds is passed but a syscall > using 32-bit time ends up being used? No. The nanoseconds check in the __utimensat_helper() function can be removed completely. Moreover, I do believe that the overflow check shall be kept to prevent calling 32 bit utimensat syscall with passed 64 bit time value in struct __timespec64. > > > + if (tsp64 && (! valid_nanoseconds(tsp64[0].tv_nsec) > > + || ! valid_nanoseconds(tsp64[1].tv_nsec))) > > Note missing space before '('. Ok. I will fix it in v2. > > > + { > > + if (tsp64[0].tv_nsec != UTIME_NOW && tsp64[0].tv_nsec != > > UTIME_OMIT > > + && tsp64[1].tv_nsec != UTIME_NOW && tsp64[1].tv_nsec != > > UTIME_OMIT) > > + { > > + __set_errno (EINVAL); > > I don't think this logic is correct; it would allow through cases > where one timestamp is invalid and the other is UTIME_NOW or > UTIME_OMIT, for example. > > It might be better to have another helper function, e.g. > valid_nanoseconds_for_utimensat, which calls valid_nanoseconds but > also accepts the UTIME_* constants. > As you stated above - this check is also performed in the Linux kernel, so it can be safely removed from the __utimensat_helper function in glibc. Best regards, Lukasz Majewski -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
Attachment:
pgpZkr_a5YMMT.pgp
Description: OpenPGP digital signature
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |