SchedMD - Slurm Support – Full Text Ticket Listing
- Home
- | New
- | Browse
- | Search
- | [?]
- | Reports
- | Help
- | New Account
- | Log In
- | Forgot Password
Thanks for assigning this. I just wanted to follow up and mention that I’m ready to rebase, retest, or adjust the patch if needed to help it along. Hello again! I started to look at this and have realized that master and 23.11 have diverged on this one. If you are able to rebase this to current master I'd really appreciate it! Thanks! --Tim I’ve started rebasing this patch onto current master. The process is taking a bit longer than expected because several affected files have been moved, split, or significantly changed since the 23.11 branch. I’m in the middle of translating the fixes into the new layout and will continue working on this rebase. I’ll report back once I have a clean patch ready for review. Rebase update: I’ve rebased the FreeBSD sockaddr length fixes onto current master. Several call sites moved into helpers (e.g., slurm_open_unix_stream) and conmgr was split, so I translated the changes to the new layout. The build is green locally. The patch ensures AF_UNIX uses SUN_LEN() + sun_len and AF_INET/AF_INET6 set sin_len/sin6_len on FreeBSD, passing the exact namelen to bind()/connect() to avoid EINVAL. Attaching freebsd-bind-fix-rebased.patch for review. Created attachment 43187 [details]
freebsd-bind-fix-rebased.patch: Normalize sockaddr length handling on FreeBSD (bind/connect)
Rebased patch against current master. Multiple files were moved or refactored since the
original version (e.g., conmgr split, stepd connect folded into slurm_open_unix_stream),
so the FreeBSD fixes were translated to the new locations.
This patch ensures:
- AF_UNIX: use SUN_LEN() and set sun_len before bind()/connect()
- AF_INET/AF_INET6: set sin_len/sin6_len and pass the correct namelen
- Avoids EINVAL on FreeBSD when binding or connecting sockets
Build tested successfully on FreeBSD. Please review for upstream inclusion.
(In reply to Rikka Göring from comment #5) > Created attachment 43187 [details] > freebsd-bind-fix-rebased.patch: Normalize sockaddr length handling on > FreeBSD (bind/connect) > > Rebased patch against current master. Multiple files were moved or > refactored since the > original version (e.g., conmgr split, stepd connect folded into > slurm_open_unix_stream), > so the FreeBSD fixes were translated to the new locations. > > This patch ensures: > - AF_UNIX: use SUN_LEN() and set sun_len before bind()/connect() > - AF_INET/AF_INET6: set sin_len/sin6_len and pass the correct namelen > - Avoids EINVAL on FreeBSD when binding or connecting sockets > > Build tested successfully on FreeBSD. Please review for upstream inclusion. Hey, thank you for the effort here! I was trying to look at this today and noticed that the file is 64 bytes in size and seems to not actually contain the patch. I think maybe its just the wrong file? (In reply to Tim McMullan from comment #6) > (In reply to Rikka Göring from comment #5) > > Created attachment 43187 [details] > > freebsd-bind-fix-rebased.patch: Normalize sockaddr length handling on > > FreeBSD (bind/connect) > > > > Rebased patch against current master. Multiple files were moved or > > refactored since the > > original version (e.g., conmgr split, stepd connect folded into > > slurm_open_unix_stream), > > so the FreeBSD fixes were translated to the new locations. > > > > This patch ensures: > > - AF_UNIX: use SUN_LEN() and set sun_len before bind()/connect() > > - AF_INET/AF_INET6: set sin_len/sin6_len and pass the correct namelen > > - Avoids EINVAL on FreeBSD when binding or connecting sockets > > > > Build tested successfully on FreeBSD. Please review for upstream inclusion. > > Hey, thank you for the effort here! > > I was trying to look at this today and noticed that the file is 64 bytes in > size and seems to not actually contain the patch. I think maybe its just > the wrong file? Hi Tim, I’m very sorry about the mix-up with the missing patch file earlier — I didn’t have the exported patch anymore, but thankfully I still had my local git branch and was able to recreate it cleanly against current master. The new attachment should now apply correctly and reflect all the intended FreeBSD bind()/connect() length fixes. If you notice anything else that needs adjusting, please don’t hesitate to let me know — that’s exactly what I’m here for! Best, Rikka Created attachment 43245 [details]
freebsd-bind-fix-rebased.patch: Normalize sockaddr length handling on FreeBSD (bind/connect)
(In reply to Rikka Göring from comment #7) > (In reply to Tim McMullan from comment #6) > > (In reply to Rikka Göring from comment #5) > > > Created attachment 43187 [details] > > > freebsd-bind-fix-rebased.patch: Normalize sockaddr length handling on > > > FreeBSD (bind/connect) > > > > > > Rebased patch against current master. Multiple files were moved or > > > refactored since the > > > original version (e.g., conmgr split, stepd connect folded into > > > slurm_open_unix_stream), > > > so the FreeBSD fixes were translated to the new locations. > > > > > > This patch ensures: > > > - AF_UNIX: use SUN_LEN() and set sun_len before bind()/connect() > > > - AF_INET/AF_INET6: set sin_len/sin6_len and pass the correct namelen > > > - Avoids EINVAL on FreeBSD when binding or connecting sockets > > > > > > Build tested successfully on FreeBSD. Please review for upstream inclusion. > > > > Hey, thank you for the effort here! > > > > I was trying to look at this today and noticed that the file is 64 bytes in > > size and seems to not actually contain the patch. I think maybe its just > > the wrong file? > > Hi Tim, > > I’m very sorry about the mix-up with the missing patch file earlier — I > didn’t have the exported patch anymore, but thankfully I still had my local > git branch and was able to recreate it cleanly against current master. > > The new attachment should now apply correctly and reflect all the intended > FreeBSD bind()/connect() length fixes. > If you notice anything else that needs adjusting, please don’t hesitate to > let me know — that’s exactly what I’m here for! > > Best, > Rikka Hi Rikka! No worries, I've done the same thing myself in the past! I'm glad you were able to re-create it cleanly with relative ease. Thanks again for this, I'm taking a look at this and will let you know how that all goes! --Tim |
Created attachment 43024 [details] Patch to fix FreeBSD bind()/connect() calls by passing correct sockaddr lengths (uses _bsd_sockaddr_len_fix() and sets sun_len where needed). No functional changes for Linux. On FreeBSD, bind() and connect() require the correct sockaddr length to be passed, including the sa_len/sun_len fields, otherwise the call may fail with EINVAL. This patch ensures the correct length is computed and applied in all affected places. The changes are minimal and guarded by #if defined(__FreeBSD__), so there is no impact on Linux or other platforms. Original patch prepared for the FreeBSD port of Slurm; refined during review by Vladimir Druzenko to minimize scope and keep diffs clean.