Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issue 2551 #3956

Closed
wants to merge 2 commits into from
Closed

Fix issue 2551 #3956

wants to merge 2 commits into from

Conversation

jboero
Copy link

@jboero jboero commented Nov 12, 2020

Kernel null dereference occasionally (race condition) when using certain webcams. Need to check before deref. Affects mainline Linux kernel too.

Fixing issue with Logitech webcams and NULL deref in kernel.
raspberrypi#2551
Fix NULL deref in kernel with certain webcams. #issue 2551
Comment on lines +279 to +280
if (!dev)
return NULL;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm really no linux hacker, but wouldn't it make sense to move this check all the way up to the opening curly brace? Initializing the struct and integer is of no use, if the device is NULL.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code is fine. You need all declarations to precede code in the C standard kernel targets.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to do that too but C best practices - declare variables first. Some compilers may have issue with the order.

@PeterPablo
Copy link

PeterPablo commented Nov 12, 2020

Did you already take your proposal upstream? This is what you'll be asked. Once it is accepted upstream, the commit could be applied here.
edit: sorry, I haven't seen the discussion that happened in #2551

@pelwell
Copy link
Contributor

pelwell commented Nov 12, 2020

As @popcornmix says, the commit itself looks fine, although we'd rather not have merge requests in PRs. As also said, we'd prefer to see it go upstream first, but if you get no response there come back and give us a nudge.

@jboero
Copy link
Author

jboero commented Nov 12, 2020

Hi yes I submitted upstream. Still waiting to hear back from the usb maintainers. Please hold on this for now. Hoping to get it upstream.

@jboero
Copy link
Author

jboero commented Nov 12, 2020

Upstream PR is here:
torvalds/linux@a405190

@6by9
Copy link
Contributor

6by9 commented Nov 12, 2020

You do realise that all mainline patches go via the mailing lists, and not as PRs to Linus' repo.

The patch subject line should also start with the subsystem that it applies to, ie "usb: core:" in this case.

https://www.kernel.org/doc/html/latest/process/submitting-patches.html

@jboero
Copy link
Author

jboero commented Nov 12, 2020

Yes sorry I'll let the mailing list deal with it. At least they can see the code changes necessary and the explanation.

@jboero jboero closed this Nov 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants