Hi, On 5/21/21 10:10 AM, Hans de Goede wrote: <snip> Some minor corrections to my last email: > This also make me realize that the code has 2 max-pwd-lengths: > > setting->maxlen > TLMI_PWD_MAXLEN > > I think you should put a: > > if (WARN_ON(pwdcfg.max_length > TLMI_PWD_MAXLEN)) > pwdcfg.max_length = TLMI_PWD_MAXLEN; This needs to be: if (WARN_ON(pwdcfg.max_length >= TLMI_PWD_MAXEN)) pwdcfg.max_length = TLMI_PWD_MAXLEN - 1; To account for the terminating 0 (maybe bump TLMI_PWD_MAXEN with 1 and rename it to TLMI_PWD_BUFSIZE?) > in tlmi_analyze() so that we get a kernel-backtrace (and thus bugreports > if this condition ever becomes true. > > And then use pwdcfg.max_length everywhere (e.g. also for the check in > current_password_store()). > > Also the length checks in current_password_store() and new_password_store() > should probably also check against settings->minlen ? I just realized that current_password_store() should also allow a length of 0 to clear the password when the user is done making changes, so the check in current_password_store() should be something like this: /* len == 0 is allowed to clear the password */ if (len != 0 && (len < setting->minlen || len > setting->maxlen)) return -EINVAL; Regards, Gans
