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: <20231001151202.GQ92317@kernel.org> Date: Sun, 1 Oct 2023 17:12:02 +0200 From: Simon Horman <horms@...nel.org> To: Xabier Marquiegui <reibax@...il.com> Cc: netdev@...r.kernel.org, richardcochran@...il.com, chrony-dev@...ony.tuxfamily.org, mlichvar@...hat.com, ntp-lists@...tcorallo.com, vinicius.gomes@...el.com, alex.maftei@....com, davem@...emloft.net, rrameshbabu@...dia.com, shuah@...nel.org Subject: Re: [PATCH net-next v3 3/3] ptp: support event queue reader channel masks On Thu, Sep 28, 2023 at 03:35:44PM +0200, Xabier Marquiegui wrote: > Implement ioctl to support filtering of external timestamp event queue > channels per reader based on the process PID accessing the timestamp > queue. > > Can be tested using testptp test binary. Use lsof to figure out readers > of the DUT. LSB of the timestamp channel mask is channel 0. > > eg: To view all current users of the device: > ``` > # testptp -F /dev/ptp0 > (USER PID) TSEVQ FILTER ID:MASK > (3234) 1:0x00000001 > (3692) 2:0xFFFFFFFF > (3792) 3:0xFFFFFFFF > (8713) 4:0xFFFFFFFF > ``` > > eg: To allow ID 1 to access only ts channel 0: > ``` > # testptp -F 1,0x1 > ``` > > eg: To allow ID 1 to access any channel: > ``` > # testptp -F 1,0xFFFFFFFF > ``` > > Signed-off-by: Xabier Marquiegui <reibax@...il.com> > Suggested-by: Richard Cochran <richardcochran@...il.com> Hi Xabier, please find some more feedback from Smatch inline. ... > diff --git a/drivers/ptp/ptp_chardev.c b/drivers/ptp/ptp_chardev.c ... > @@ -169,19 +170,28 @@ long ptp_ioctl(struct posix_clock_user *pcuser, unsigned int cmd, > { > struct ptp_clock *ptp = > container_of(pcuser->clk, struct ptp_clock, clock); > + struct ptp_tsfilter tsfilter_set, *tsfilter_get = NULL; > struct ptp_sys_offset_extended *extoff = NULL; > struct ptp_sys_offset_precise precise_offset; > struct system_device_crosststamp xtstamp; > struct ptp_clock_info *ops = ptp->info; > struct ptp_sys_offset *sysoff = NULL; > + struct timestamp_event_queue *tsevq; > struct ptp_system_timestamp sts; > struct ptp_clock_request req; > struct ptp_clock_caps caps; > struct ptp_clock_time *pct; > + int lsize, enable, err = 0; > unsigned int i, pin_index; > struct ptp_pin_desc pd; > struct timespec64 ts; > - int enable, err = 0; > + > + tsevq = pcuser->private_clkdata; > + > + if (tsevq->close_req) { > + err = -EPIPE; > + return err; > + } Here tseqv is dereferenced unconditionally... > > switch (cmd) { > > @@ -481,6 +491,79 @@ long ptp_ioctl(struct posix_clock_user *pcuser, unsigned int cmd, > mutex_unlock(&ptp->pincfg_mux); > break; > > + case PTP_FILTERCOUNT_REQUEST: > + /* Calculate amount of device users */ > + if (tsevq) { ... but here it is assumed that tseqv might be NULL. As flagged by Smatch. > + lsize = list_count_nodes(&tsevq->qlist); > + if (copy_to_user((void __user *)arg, &lsize, > + sizeof(lsize))) > + err = -EFAULT; > + } > + break; > + case PTP_FILTERTS_GET_REQUEST: > + /* Read operation */ > + /* Read amount of entries expected */ > + if (copy_from_user(&tsfilter_set, (void __user *)arg, > + sizeof(tsfilter_set))) { > + err = -EFAULT; > + break; > + } > + if (tsfilter_set.ndevusers <= 0) { > + err = -EINVAL; > + break; > + } > + /* Allocate the necessary memory space to dump the requested filter > + * list > + */ > + tsfilter_get = kzalloc(tsfilter_set.ndevusers * > + sizeof(struct ptp_tsfilter), > + GFP_KERNEL); > + if (!tsfilter_get) { > + err = -ENOMEM; > + break; > + } > + if (!tsevq) { Ditto. > + err = -EFAULT; > + break; > + } > + /* Set the whole region to 0 in case the current list is shorter than > + * anticipated > + */ > + memset(tsfilter_get, 0, > + tsfilter_set.ndevusers * sizeof(struct ptp_tsfilter)); > + i = 0; > + /* Format data */ > + list_for_each_entry(tsevq, &ptp->tsevqs, qlist) { > + tsfilter_get[i].reader_rpid = tsevq->reader_pid; > + tsfilter_get[i].reader_oid = tsevq->oid; > + tsfilter_get[i].mask = tsevq->mask; > + i++; > + /* Current list is longer than anticipated */ > + if (i >= tsfilter_set.ndevusers) > + break; > + } > + /* Dump data */ > + if (copy_to_user((void __user *)arg, tsfilter_get, > + tsfilter_set.ndevusers * > + sizeof(struct ptp_tsfilter))) > + err = -EFAULT; > + break; > + > + case PTP_FILTERTS_SET_REQUEST: > + /* Write Operation */ > + if (copy_from_user(&tsfilter_set, (void __user *)arg, > + sizeof(tsfilter_set))) { > + err = -EFAULT; > + break; > + } > + if (tsevq) { Ditto. > + list_for_each_entry(tsevq, &ptp->tsevqs, qlist) { > + if (tsevq->oid == tsfilter_set.reader_oid) > + tsevq->mask = tsfilter_set.mask; > + } > + } > + break; > + > default: > err = -ENOTTY; > break; ...
Powered by blists - more mailing lists