pkarashchenko commented on a change in pull request #5204:
URL: https://github.com/apache/incubator-nuttx/pull/5204#discussion_r819914782
##########
File path: drivers/input/ajoystick.c
##########
@@ -526,12 +461,7 @@ static ssize_t ajoy_read(FAR struct file *filep, FAR char
*buffer,
/* Get exclusive access to the driver structure */
- ret = ajoy_takesem(&priv->au_exclsem);
- if (ret < 0)
- {
- ierr("ERROR: ajoy_takesem failed: %d\n", ret);
- return ret;
- }
+ flags = enter_critical_section();
Review comment:
The current PR does not reflect the actual code and should be rebased,
so I will write a comment taking into assumption that PR is based on the latest
mainline.
I'm not sure if what I describe is a valid use case, so please correct me if
I'm wrong.
On the latest mainline the next code is present:
```
lower = priv->au_lower;
DEBUGASSERT(lower && lower->al_sample);
ret = lower->al_sample(lower, (FAR struct ajoy_sample_s *)buffer);
if (ret >= 0)
{
opriv->ao_pollpending = false;
ret = sizeof(struct ajoy_sample_s);
}
```
So I'm thinking if that can be a situation when we enter a critical section
and then `al_sample` pends. While we are waiting in `al_sample` the
`ajoy_close` is called and that leads to `kmm_free(opriv);` so after we return
from `al_sample` the `opriv->ao_pollpending = false;` can access released
memory.
In general I do not understand what we protect here with a critical section?
Maybe critical section can be removed at all from `ajoy_read`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
