Page MenuHomeFreeBSD
Differential D35605

ipmi: handle ipmb requests
ClosedPublic
Actions

Authored by yuri_aetern.org on Jun 26 2022, 1:22 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Apr 22, 7:38 AM
Unknown Object (File)
Fri, Apr 19, 5:11 PM
Unknown Object (File)
Fri, Apr 19, 5:11 PM
Unknown Object (File)
Fri, Apr 19, 5:11 PM
Unknown Object (File)
Fri, Apr 19, 5:11 PM
Unknown Object (File)
Wed, Apr 17, 3:33 AM
Unknown Object (File)
Mon, Apr 1, 1:53 AM
Unknown Object (File)
Jan 29 2024, 8:52 AM
View All 55 Files
Subscribers
avg
imp
philip
vangyzen

Details

Reviewers
philip
Commits
rG1549a6172f9d: ipmi: correctly handle ipmb requests
rGc4995b69db93: ipmi: fix a use-after-free bug in error handling
rG18db96dbfd4a: ipmi: correctly handle ipmb requests
Summary

Handle IPMB requests using SEND_MSG (sent as driver request as we do not need to return anything back to userland for this) and GET_MSG (sent as usual request so we can return the data for RECEIVE_MSG ioctl) pair.

Test Plan

Tested using ipmitool on HPE ProLiant DL380 Gen10 now properly showing all of the sensors.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

yuri_aetern.org created this revision.Jun 26 2022, 1:22 PM
Herald added a subscriber: imp. · View Herald TranscriptJun 26 2022, 1:22 PM
yuri_aetern.org requested review of this revision.Jun 26 2022, 1:22 PM
avg added a subscriber: avg.Jun 26 2022, 6:37 PM
yuri_aetern.org added inline comments.Jun 26 2022, 7:05 PM
sys/dev/ipmi/ipmi.c
388

this removal is likely unrelated and could be done separately, just a note that we are using the ipmi driver in illumos ported from FreeBSD with this part removed for *years*, so the more wide testing of this change is based on the assumption that this is removed

philip requested changes to this revision.Jul 3 2022, 5:48 AM
philip added a subscriber: philip.

I've tested this on a DL380 and can confirm it works.

I don't like the #if 0 bit. Other than that, I think this can be committed.

(I don't like the mix of u_char and uint8_t in this driver either, but that's a separate issue.)

sys/dev/ipmi/ipmi.c
251–252

Just delete it instead of #if 0 bracketing it. If we need to resurrect this code, we can get it back from Git.

This revision now requires changes to proceed.Jul 3 2022, 5:48 AM
yuri_aetern.org updated this revision to Diff 107701.Jul 3 2022, 4:52 PM

removed the ifdef 0'ed code

yuri_aetern.org marked an inline comment as done.Jul 3 2022, 4:52 PM
This revision was not accepted when it landed; it landed in state Needs Review.Jul 4 2022, 5:33 AM
Closed by commit rG18db96dbfd4a: ipmi: correctly handle ipmb requests (authored by yuri_aetern.org, committed by philip). · Explain Why
This revision was automatically updated to reflect the committed changes.
philip added a commit: rG18db96dbfd4a: ipmi: correctly handle ipmb requests.
philip mentioned this in D35612: ipmi: do not omit lun in BMC addresses.Jul 4 2022, 5:34 AM
vangyzen added a subscriber: vangyzen.Jul 7 2022, 10:58 PM
vangyzen added inline comments.
sys/dev/ipmi/ipmi.c
393

This uses kreq after freeing it. Coverity CID 1490456

philip added inline comments.Jul 8 2022, 2:42 AM
sys/dev/ipmi/ipmi.c
393

Good catch! Thanks. I'll commit this fix.

philip added a commit: rGc4995b69db93: ipmi: fix a use-after-free bug in error handling.Jul 8 2022, 3:51 AM
yuri_aetern.org added a comment.Jul 8 2022, 5:20 AM

My bad, thanks for fixing this.

philip added a commit: rG1549a6172f9d: ipmi: correctly handle ipmb requests.Jul 11 2022, 3:16 AM

Revision Contents
Changeset List

PathSize
sys/
dev/
ipmi/
185 lines
13 lines
sys/
4 lines

Diff 107717

View Options

sys/dev/ipmi/ipmi.c

Loading...
View Options

sys/dev/ipmi/ipmivars.h

Loading...
View Options

sys/sys/ipmi.h

Loading...