Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

warning messages requested by the BIOS team #20

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

warning messages requested by the BIOS team #20

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
af7e550
warning messages requested by the BIOS team
sugudsl Oct 22, 2021
0bb6939
Merge branch 'lenovo:master' into master
sugudsl Dec 16, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 30 additions & 15 deletions thinklmi-user/thinklmi.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include <string.h>
#include <sys/ioctl.h>
#include <stdlib.h>
#include <ctype.h>

#include "../thinklmi-kernel/think-lmi.h"

Expand Down Expand Up @@ -66,15 +67,22 @@ void thinklmi_get(int fd, char * argv2)
void thinklmi_set(int fd, char * argv2, char* argv3)
{
char setting_string[TLMI_GETSET_MAXLEN];
strncpy(setting_string, argv2, TLMI_SETTINGS_MAXLEN);
strcat(setting_string, ",");
strncat(setting_string, argv3, TLMI_SETTINGS_MAXLEN);
char option;

if(ioctl(fd, THINKLMI_SET_SETTING, &setting_string) == -1) {
perror("Unable to change setting");
} else {
printf("BIOS Setting changed\n");
printf("Setting will not change until reboot\n");
printf("Please check whether the setting name is Valid\n");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lower case v in Valid

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

corrected

printf("please complete authentication if BIOS password is set\n");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Capital P in please

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Corrected

printf("Do you want to continue (Y/N):");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I personally think the first 'warning' is pointless. If they're not using a valid name the call will fail, and they're not going to use an invalid name deliberately.
I would put the authentication warning if the set fails as a possible reason it fails. But it would be much better to probe the interface to confirm if admin password is set and to print this message if it's required. Blindly printing it everytime is just annoying.
Consider adding a flag to skip the continue check - adding this will make scripting thinklmi harder.

Copy link
Collaborator Author

@sugudsl sugudsl Mar 15, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed the first warning.
Probing and printing the warning only if the password is set, will take it as an improvement for the next release.
Removed the Y/N check for setting the setting. Retained for the TPM type change

scanf("%c", &option);
if(tolower(option) == 'y' && tolower(option) != 'n') {
strncpy(setting_string, argv2, TLMI_SETTINGS_MAXLEN);
strcat(setting_string, ",");
strncat(setting_string, argv3, TLMI_SETTINGS_MAXLEN);
if(ioctl(fd, THINKLMI_SET_SETTING, &setting_string) == -1) {
perror("Unable to change setting");
} else {
printf("BIOS Setting changed\n");
printf("Setting will not change until reboot\n");
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your indentation in the above looks wonky - check your editor settings.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My vim editor displays this correctly, however there a tab, which I removed

}
}

Expand Down Expand Up @@ -132,14 +140,21 @@ void thinklmi_lmiopcode(int fd, char *admin, char *passtype, char *oldpass, char
void thinklmi_tpmtype(int fd, char *tpmtype)
{
char setting_string[TLMI_GETSET_MAXLEN];
snprintf(setting_string, TLMI_GETSET_MAXLEN, "%s;", tpmtype);
if(ioctl(fd, THINKLMI_TPMTYPE, &setting_string) == -1) {
perror("Tpm type change failed");
} else {
printf("Tpm type changed\n");
printf("Setting will not change until reboot\n");
}
char option;

printf("Before switching TPM, please make sure the TPM is not in use and all TPM related applications\n");
printf("must be disabled, otherwise the TPM will be cleared and you may not be able to access your data\n");
printf("\n Do you wish to continue(Y/N):");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As above - consider adding a nocheck option to bypass the continue check for easier scripting

scanf("%c", &option);
if(tolower(option) == 'y' && tolower(option) != 'n') {
snprintf(setting_string, TLMI_GETSET_MAXLEN, "%s;", tpmtype);
if(ioctl(fd, THINKLMI_TPMTYPE, &setting_string) == -1) {
perror("Tpm type change failed");
} else {
printf("Tpm type changed\n");
printf("Setting will not change until reboot\n");
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tpm in the above perrors should be capitalised (TPM)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Corrected

}
}

void thinklmi_load_default(int fd)
Expand Down