ghaerr / elks

Embeddable Linux Kernel Subset - Linux for 8086
Other
999 stars 108 forks source link

Cannot set passwd to user account #2041

Closed tyama501 closed 2 days ago

tyama501 commented 4 days ago

Hi @ghaerr

It seems that something is wrong with passwd command in ELKS 0.8.0.

When I login with user1 and do passwd the /etc/passwd becomes the file only with user2 line.

ELKS_0 8 0_passwd

When I copy the last user2 line and create another account tyama, login with it and do passwd, the /etc/passwd becomes empty.

ELKS_0 8 0_passwd2

I am using FAT file system and removed comment out in /etc/profile if test "$USER" = "root"; then chmod 777 /tmp; fi.

Could you take a look when you are back? Thank you.

ghaerr commented 3 days ago

Hello @tyama501,

Geez! Something is definitely wrong with passwd. I have it duplicating here on MINIX filesystem as well. I'm not yet sure what the problem is, perhaps it may be something due to recent changes in C library, I am not sure yet. I will look into this and provide a fix ASAP. I am so sorry we have found this immediately after a release. We may have to release an v8.0.1 unless a good workaround can be given.

Thank you for your testing!

ghaerr commented 3 days ago

Hello @tyama501,

I've found the problem. Looks like this has been broken since the C library getpwent routines were enhanced for speed in #1690 over a year ago! I will open a PR to fix but am in the middle of another development.

Please try this one line diff change to confirm this solves the problem:

diff --git a/elkscmd/sys_utils/passwd.c b/elkscmd/sys_utils/passwd.c
index 343aab20..03a9a03c 100644
--- a/elkscmd/sys_utils/passwd.c
+++ b/elkscmd/sys_utils/passwd.c
@@ -113,6 +113,7 @@ main(int argc, char **argv)
             uid = pwd->pw_uid;

             /* save entries to the new file */
+            setpwent();
             pwd = getpwent();
             while (pwd != NULL) {
                 if (pwd->pw_uid == uid) {

Thank you!

tyama501 commented 3 days ago

Thank you.

I will try it.

BTW, passwd in ELKS 0.7.0 image still can be used for the ELKS 0.8.0?

I will inform in my website to use it for now if it can be used. https://tyama501.github.io/elks98doc/

ghaerr commented 3 days ago

passwd in ELKS 0.7.0 image still can be used for the ELKS 0.8.0?

Yes, that's a good idea.

I will inform in my website to use it for now if it can be used.

Ok. Go ahead and play with the release version a bit more. I will release a v0.8.1 after that if we find nothing else, so that your users have a fully working version.

tyama501 commented 3 days ago

Hi @ghaerr .

Adding setpwent worked. Here is the compiled binary. passwd.zip

Maybe I will write this link to the website for now.

I also tried using the 0.7.0 passwd but once I failed to remove temp file. I am not sure this is related to unlink issue we have solved or not. Well, I tried again before setting root passwd, it worked but I recommend above new one for now. remove_tmp

Thank you.

ghaerr commented 3 days ago

I also tried using the 0.7.0 passwd but once I failed to remove temp file. I am not sure this is related to unlink issue we have solved or not.

Are you saying that the fixed passwd now always gets a "Permission denied" error when trying to remove the old passwd temp file? Or did that only happen once?

The problem could have to do with /tmp not having write permissions on FAT filesystem, which is what the if test "$USER" = "root"; then chmod 777 /tmp; fi. is supposed to fix. Can you ls -ld /tmp and see what the permissions are?

We could potentially fix this in a better way by creating the temporary file in the current directory, rather than /tmp. I will have to look into this (again), it seems that we probably want to have ELKS give all directories mode 777, rather than 755. I am trying to remember the reason we don't use 777 now for all files on FAT, rather than 755. The bigger issue is that FAT filesystem doesn't have permissions, so ELKS has to kind of fake them.

tyama501 commented 3 days ago

No, Fixed passwd worked well. The above attached is the fixed passwd.

Yes the if... worked well with the above new passwd.

ghaerr commented 3 days ago

Yes the if... worked well with the above new passwd.

But I still see the "Error removing old password file" message... Is that happening with the if ... fix? It should not be.

tyama501 commented 3 days ago

That picture is using 0.7.0 passwd with 0.8.0 kernel. (Well, I haven't seen that error with 0.8.0 fixed passwd yet)

ghaerr commented 3 days ago

Ok, thank you. Yes - the new passwd fixed the problem that v0.7.0 passwd had when "renaming" the file to use mv instead for FAT. So all that needs to be done is the 0.8.1 fix adding setpwent, which is coming.

tyama501 commented 3 days ago

Well, this is different issue but nxtetris for PC-98 is shrank to CGA size...

image

ghaerr commented 3 days ago

this is different issue but nxtetris for PC-98 is shrank to CGA size...

Are you thinking this is an improper build issue for PC-98? The default build should not be for any CGA, right?

tyama501 commented 3 days ago

https://github.com/ghaerr/elks/blob/master/elkscmd/nano-X/demos/ntetris.h#L57

This should be ifdef CONFIG_HW_VGA or ifdef CONFIG_ARCH_PC98...

tyama501 commented 3 days ago

Updated the site for 0.8.0. https://tyama501.github.io/elks98doc/

I haven't seen other issues. Thanks.

ghaerr commented 2 days ago

@tyama501, for this last issue, go ahead and submit a PR to fix if you can find what the problem is, thank you!

tyama501 commented 2 days ago

Hi @ghaerr ,

I don't know why but ifdef CONFIG_HW_VGA in the ntetris.h does not work. Released 0.8.0 nxtetris in IBM image fd1440-fat.img also has been shrank.

image

I modified the line to if defined(CONFIG_HW_VGA) || defined(CONFIG_ARCH_PC98) and it worked for PC-98 but the IBM vga is still shrank.

tyama501 commented 2 days ago

I thought I checked before but this is very strange...

ghaerr commented 2 days ago

@tyama501, I'm looking at this too and don't see why the problem...

Actually I'm not really keen on the idea of having to recompile applications for differing platforms, including differences in basic or other text-mode applications. Later versions of Microwindows have support for multiple framebuffer formats built into a single driver, called subdrivers. We will have to check if our early version of Microwindows allows this, or add it, with the idea that a single application would run on CGA vs EGA without recompilation. We can still keep separate compilation for PC-98, although it might be possible for that driver to be included as well.

All this is kind of complicated, its probably better to consider the issues with basic and/or CGA vs EGA and screen sizes with non-Nano-X programs first, as well as come up with a better mechanism how/when to size the screen?

Good news is that the smaller size doesn't look too bad, at least for now!

tyama501 commented 2 days ago

OK, let me change it to just ifdef CONFIG_ARCH_PC98 later since it works good.

tyama501 commented 2 days ago

Sorry @ghaerr ,

Maybe I was a little tired. The vga nxtetris is not shrank and the CONFIG_HW_VGA is working.

It looks small than PC-98 because the VGA is 480 lines while the PC-98 only has 400 lines.

I will make a PR for PC-98.