| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <e2f96f41643f4295812f43e801c8d70e@diehl.com>
Date: Fri, 21 Feb 2025 11:54:42 +0000
From: Denis OSTERLAND-HEIM <denis.osterland@...hl.com>
To: Rodolfo Giometti <giometti@...eenne.com>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: Re: [PATCH] pps: add epoll support
Hi,
Thanks!
I will test your suggentions and report next week ;-)
Have a nice weekend.
Regards, Denis
-----Original Message-----
From: Rodolfo Giometti <giometti@...eenne.com>
Sent: Friday, February 21, 2025 12:40 PM
To: Denis OSTERLAND-HEIM <denis.osterland@...hl.com>
Cc: linux-kernel@...r.kernel.org
Subject: Re: [PATCH] pps: add epoll support
On 21/02/25 11:49, Denis OSTERLAND-HEIM wrote:
> Hi,
>
> Okay, if poll is expected to work, than we have a bug.
> Actually a pretty old one.
>
> pps_cdev_poll() uncoditionally returns (EPOLLIN | EPOLLRDNORM), which results in poll() will return immediately with data available
> (EPOLLIN | EPOLLRDNORM).
> To avoid this, you need conditionally return 0.
I think you are right!
Looking at the code I think the correct patch should be:
diff --git a/drivers/pps/pps.c b/drivers/pps/pps.c
index 6a02245ea35f..7a52bb9f835b 100644
--- a/drivers/pps/pps.c
+++ b/drivers/pps/pps.c
@@ -41,7 +41,7 @@ static __poll_t pps_cdev_poll(struct file *file, poll_table *wait)
poll_wait(file, &pps->queue, wait);
- return EPOLLIN | EPOLLRDNORM;
+ return pps->info.mode & PPS_CANWAIT ? 0 : EPOLLIN | EPOLLRDNORM;
}
static int pps_cdev_fasync(int fd, struct file *file, int on)
> My patch adds a context per open file to store the last_ev value when ioctl(PPS_FETCH) is invoked and uses this last_ev in poll as
> condition.
>
> Sorry, for the missing memset(&fdata, 0, sizeof(fdata)).
> Intention was set to 0, yes.
OK
> ```c
> #include <stdio.h>
> #include <string.h>///home/giometti/Projects/ailux/imx9/linux/linux-imx
> #include <poll.h>
> #include <fcntl.h>
> #include "timepps.h"
>
> int main(int argc, const char* argv[]) {
> struct pollfd instance = { .fd = open((argc > 1) ? argv[1] : "/dev/pps0", O_RDONLY), .events = POLLIN|POLLERR , .revents = 0 };
> pps_handle_t pps_handle;
> static const struct timespec timeout = { 0, 0 };
> if (time_pps_create(instance.fd, &pps_handle)) {
> perror("failed to create pps handle");
> return 1;
> }
> for (int loops = 4; --loops; ) {
> pps_info_t pps_info;
> memset(&pps_info, 0, sizeof(pps_info));
> if (!poll(&instance, 1, 2000/*ms*/)) {
> printf("timeout");
> continue;
> }
> if ((instance.revents & POLLIN) != POLLIN) {
> printf("nothing to read?");
> continue;
> }
> if (time_pps_fetch(pps_handle, PPS_TSFMT_TSPEC, &pps_info, &timeout)) {
> perror("failed to fetch");
> return 1;
> }
>
> printf(
> "assert: %lu\ntime: %ld.%09ld\n",
> pps_info.assert_sequence,
> pps_info.assert_tu.tspec.tv_sec,
> pps_info.assert_tu.tspec.tv_nsec
> );
> }
> return 0;
> }
> ```
>
> Currently output looks like:
> ```
> $ cat /sys/class/pps/pps0/assert; ./test /dev/pps0
> 1520598954.468882076#60
> assert: 60
> time: 1520598954.468882076
> assert: 60
> time: 1520598954.468882076
> assert: 60
> time: 1520598954.468882076
> ```
>
> You see no waits between the loops.
Please, try again with the above patch.
However, before doing the test, you should consider to add this patch too:
diff --git a/drivers/pps/pps.c b/drivers/pps/pps.c
index 6a02245ea35f..7a52bb9f835b 100644
--- a/drivers/pps/pps.c
+++ b/drivers/pps/pps.c
@@ -56,10 +56,13 @@ static int pps_cdev_pps_fetch(struct pps_device *pps, struct
pps_fdata *fdata)
int err = 0;
/* Manage the timeout */
- if (fdata->timeout.flags & PPS_TIME_INVALID)
- err = wait_event_interruptible(pps->queue,
+ if (fdata->timeout.flags & PPS_TIME_INVALID) {
+ if (pps->info.mode & PPS_CANWAIT)
+ err = wait_event_interruptible(pps->queue,
ev != pps->last_ev);
- else {
+ else
+ return -EOPNOTSUPP;
+ } else {
unsigned long ticks;
dev_dbg(&pps->dev, "timeout %lld.%09d\n",
@@ -69,12 +72,15 @@ static int pps_cdev_pps_fetch(struct pps_device *pps, struct
pps_fdata *fdata)
ticks += fdata->timeout.nsec / (NSEC_PER_SEC / HZ);
if (ticks != 0) {
- err = wait_event_interruptible_timeout(
+ if (pps->info.mode & PPS_CANWAIT) {
+ err = wait_event_interruptible_timeout(
pps->queue,
ev != pps->last_ev,
ticks);
- if (err == 0)
- return -ETIMEDOUT;
+ if (err == 0)
+ return -ETIMEDOUT;
+ } else
+ return -EOPNOTSUPP;
}
}
In fact RFC2783 states:
3.4.3 New functions: access to PPS timestamps
...
Support for blocking behavior is an implementation option. If the
PPS_CANWAIT mode bit is clear, and the timeout parameter is either
NULL or points to a non-zero value, the function returns an
EOPNOTSUPP error. An application can discover whether the feature is
implemented by using time_pps_getcap() to see if the PPS_CANWAIT mode
bit is set.
...
Ciao,
Rodolfo
--
GNU/Linux Solutions e-mail: giometti@...eenne.com
Linux Device Driver giometti@...ux.it
Embedded Systems phone: +39 349 2432127
UNIX programming
Diehl Metering GmbH, Donaustrasse 120, 90451 Nuernberg
Sitz der Gesellschaft: Ansbach, Registergericht: Ansbach HRB 69
Geschaeftsfuehrer: Dr. Christof Bosbach (Sprecher), Dipl.-Dolm. Annette Geuther, Dipl.-Kfm. Reiner Edel, Jean-Claude Luttringer
Bitte denken Sie an die Umwelt, bevor Sie diese E-Mail drucken. Diese E-Mail kann vertrauliche Informationen enthalten. Sollten die in dieser E-Mail enthaltenen Informationen nicht für Sie bestimmt sein, informieren Sie bitte unverzueglich den Absender per E-Mail und loeschen Sie diese E-Mail in Ihrem System. Jede unberechtigte Form der Reproduktion, Bekanntgabe, Aenderung, Verteilung und/oder Publikation dieser E-Mail ist strengstens untersagt. Informationen zum Datenschutz finden Sie auf unserer Homepage<https://www.diehl.com/metering/de/impressum-und-rechtliche-hinweise/>.
Before printing, think about environmental responsibility.This message may contain confidential information. If you are not authorized to receive this information please advise the sender immediately by reply e-mail and delete this message without making any copies. Any form of unauthorized use, publication, reproduction, copying or disclosure of the e-mail is not permitted. Information about data protection can be found on our homepage<https://www.diehl.com/metering/en/data-protection/>.
Powered by blists - more mailing lists