On 5/27/2025 4:49 AM, Su Hui wrote:
kstrtoul() is better because simple_strtoul() ignores overflow which may lead to unexpected results. Signed-off-by: Su Hui<suhui@xxxxxxxxxxxx> --- fs/afs/addr_prefs.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/fs/afs/addr_prefs.c b/fs/afs/addr_prefs.c index c0384201b8fe..ae4f4b371882 100644 --- a/fs/afs/addr_prefs.c +++ b/fs/afs/addr_prefs.c @@ -118,7 +118,10 @@ static int afs_parse_address(char *p, struct afs_addr_preference *pref)if (*p == '/') {p++; - tmp = simple_strtoul(p, &p, 10); + if (kstrtoul(p, 10, &tmp)) { + pr_warn("Invalid address\n"); + return -EINVAL; + } if (tmp > mask) { pr_warn("Subnet mask too large\n"); return -EINVAL; @@ -130,11 +133,6 @@ static int afs_parse_address(char *p, struct afs_addr_preference *pref) mask = tmp; }- if (*p) {- pr_warn("Invalid address\n"); - return -EINVAL; - } - pref->subnet_mask = mask; return 0; }
Su Hui, Thank you for the contribution but I do not believe this patch is correct.The second block is required even if the simple_stroul() is replaced by kstrtoul() as it protects against an input string which does not contain the optional subnet mask but has some other characters after the address.
afs_parse_address() already has its own overflow checks following the simple_strtoul() call which is specific to the interpretation of the allowed subnet mask values.
Do you see an overflow condition which would not be caught by those checks which would be caught by use of kstrtoul()?
Thanks again. Jeffrey Altman
Attachment:
smime.p7s
Description: S/MIME Cryptographic Signature
