pru & stat(2) failure / return cleanups

[Tyler Retzlaff]

hello,

recently effort is being undertaken to separate out functions being
dispatched through the generic pr_usrreq() switches. as a result some
inconsistencies have been identified in failure / return of PRU_SENSE
requests (i.e. stat(2)).

issue #1 so_pcb is NULL

across the range of protocols there is inconsistency in whether or not
the so_pcb being NULL is an error.

for protocols that allocate a pcb in attach should it ever be valid
for socket->so_pcb to be NULL (except upon entry to attach)?

currently only tcp & rip6 deviate from this expectation. tcp because of
pr/46077 (which made PRU_SENSE not fail) and rip6 is probably just wrong.


issue #2 PRU_SENSE / stat(2) returning success, a lot

most implementations of pr_stat don't fill any values / change the
passed in struct stat *ub in any way but some of them return 0
(success) and some of them return EOPNOTSUPP.

i'd like to suggest that for all the cases where struct stat *ub is
not being filled in they be changed to EOPNOTSUPP because it seems
a bit wrong to do nothing at all and then claim success.


comments, clarification, education invited

thanks

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de

[Iain Hibbert]

Post by Tyler Retzlaff
hello,
recently effort is being undertaken to separate out functions being
dispatched through the generic pr_usrreq() switches. as a result some
inconsistencies have been identified in failure / return of PRU_SENSE
requests (i.e. stat(2)).
issue #1 so_pcb is NULL
across the range of protocols there is inconsistency in whether or not
the so_pcb being NULL is an error.
for protocols that allocate a pcb in attach should it ever be valid
for socket->so_pcb to be NULL (except upon entry to attach)?
currently only tcp & rip6 deviate from this expectation. tcp because of
pr/46077 (which made PRU_SENSE not fail) and rip6 is probably just wrong.

I do not remember exactly why, but in the Bluetooth (sys/netbt/) socket
code, error is only returned for (so_pcb == NULL) after PRU_CONTROL,
PRU_PURGEIF and PRU_ATTACH are handled. It probably means that I found
somewhere that called those functions without a pcb .. at least
PRU_CONTROL and PRU_PURGEIF are reached through ioctl() I don't know if
that is relevant

Post by Tyler Retzlaff
issue #2 PRU_SENSE / stat(2) returning success, a lot
most implementations of pr_stat don't fill any values / change the
passed in struct stat *ub in any way but some of them return 0
(success) and some of them return EOPNOTSUPP.
i'd like to suggest that for all the cases where struct stat *ub is
not being filled in they be changed to EOPNOTSUPP because it seems
a bit wrong to do nothing at all and then claim success.

seems ok to me

regards,
iain

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de

[Tyler Retzlaff]

hi,

thanks for the reply

Post by Iain Hibbert

Post by Tyler Retzlaff
for protocols that allocate a pcb in attach should it ever be valid
for socket->so_pcb to be NULL (except upon entry to attach)?

I do not remember exactly why, but in the Bluetooth (sys/netbt/) socket
code, error is only returned for (so_pcb == NULL) after PRU_CONTROL,
PRU_PURGEIF and PRU_ATTACH are handled. It probably means that I found
somewhere that called those functions without a pcb .. at least
PRU_CONTROL and PRU_PURGEIF are reached through ioctl() I don't know if
that is relevant

okay, for the netbt protocols i'll make sure to retain existing
behavior. i didn't mention PRU_PURGEIF but you're right the existing
code does handle it differently in advance of testing so_pcb.

for those requests that aren't PRU_CONTROL, PRU_PURGEIF, PRU_ATTACH we
will likely go as far as KASSERT that so_pcb is not NULL. i'm not sure if
the gallery has any thoughts on doing so?

tyler

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de

[Iain Hibbert]

Post by Tyler Retzlaff

Post by Iain Hibbert

Post by Tyler Retzlaff
for protocols that allocate a pcb in attach should it ever be valid
for socket->so_pcb to be NULL (except upon entry to attach)?

I do not remember exactly why, but in the Bluetooth (sys/netbt/) socket
code, error is only returned for (so_pcb == NULL) after PRU_CONTROL,
PRU_PURGEIF and PRU_ATTACH are handled. It probably means that I found
somewhere that called those functions without a pcb .. at least
PRU_CONTROL and PRU_PURGEIF are reached through ioctl() I don't know if
that is relevant

okay, for the netbt protocols i'll make sure to retain existing
behavior.

please don't provide special handling for any protocol family; PRU_PURGEIF
at least is unused with the Bluetooth protocol. I'm just pointing out that
it might be possible for them to pass a NULL so_pcb, since code handles
them before it tests for that.

Post by Tyler Retzlaff
i didn't mention PRU_PURGEIF but you're right the existing
code does handle it differently in advance of testing so_pcb.

it is only called, that I can see by if_detach() in net/if.c, and so_pcb
will definitely be NULL in that case. To be honest, this is an invasion of
the usrreq for some unrelated task IMO .. just provide another function
for this, that does not require a fake socket pointer.

Post by Tyler Retzlaff
for those requests that aren't PRU_CONTROL, PRU_PURGEIF, PRU_ATTACH we
will likely go as far as KASSERT that so_pcb is not NULL. i'm not sure if
the gallery has any thoughts on doing so?

If you examine all the calling paths and ensure that a user *cannot* pass
a NULL so_pcb to these functions then it seems ok to me.. otherwise it
should really return EINVAL as before

iain

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de