Bug 3291

Summary: support process_extended_limits() only if HAVE_GETRLIMIT is set
Product: Portable OpenSSH Reporter: balu <balu.gajjala>
Component: sftp-serverAssignee: Assigned to nobody <unassigned-bugs>
Status: CLOSED FIXED    
Severity: normal CC: balu.gajjala, djm
Priority: P5    
Version: 8.5p1   
Hardware: Other   
OS: Windows 10   
Bug Depends on:    
Bug Blocks: 3270    

Description balu 2021-04-07 04:38:34 AEST
Not all OS distros support rlimit/slimit so sftp-server should support process_extended_limits() only when HAVE_GETRLIMIT is defined/set.

https://github.com/openssh/openssh-portable/blob/0727dd09eca355e7539cbcb23b148fcee9b21513/sftp-server.c#L1450

https://github.com/openssh/openssh-portable/blob/0727dd09eca355e7539cbcb23b148fcee9b21513/sftp-server.c#L160
Comment 1 Damien Miller 2021-04-07 08:22:22 AEST
I have wrapped the getlimit() call in HAVE_GETRLIMIT in commit f283a6c2e, but I don't see any reason to skip offering limits extension - it does offer additional limits beyond the one based on getrlimit()
Comment 2 balu 2021-04-07 08:49:27 AEST
Thanks for your reply.
I saw your commit f283a6c2e.

one comment, 
ifdef HAVE_GETRLIMIT should be moved before struct rlimit rlim;

After this change, on the machines where the rlimit is not supported, the nfiles (max-open-handles) will be sent as 0. How will the sftp client behave when sftp-server sends nfiles as 0?
Comment 3 Damien Miller 2021-04-07 08:52:54 AEST
limits.(In reply to balu from comment #2)
> Thanks for your reply.
> I saw your commit f283a6c2e.
> 
> one comment, 
> ifdef HAVE_GETRLIMIT should be moved before struct rlimit rlim;

done
 
> After this change, on the machines where the rlimit is not
> supported, the nfiles (max-open-handles) will be sent as 0. How will
> the sftp client behave when sftp-server sends nfiles as 0?

Per PROTOCOL:

If the server doesn't enforce a specific limit, then the field may be
set to 0.  This implies the server relies on the OS to enforce limits
(e.g. available memory or file handles), and such limits might be
dynamic.  The client SHOULD take care to not try to exceed reasonable
limits.
Comment 4 balu 2021-04-07 08:56:31 AEST
Thank you for fixing it in no time.
Comment 5 Damien Miller 2021-04-23 15:10:44 AEST
closing resolved bugs as of 8.6p1 release
Comment 6 Damien Miller 2021-04-23 15:10:47 AEST
closing resolved bugs as of 8.6p1 release