Bug 698532

BTW, the above testing was run on a virtual machine of VMware workstation.
I also tried the same steps on a real machine running Fedora 14:
CPU: Intel(R) Pentium(R) 4 CPU 2.60GHz
VGA card: Intel Corporation 82865G Integrated Graphics Controller (rev 02)
This issue is reproducible.

Oops.. 
"If jfbterm is run run in screen, it will hang and won't back to command line
prompt." -> I mean: 
"If jfbterm is _not_ run in screen, it will hang and won't back to command line
prompt."

Created attachment 494812 [details]
Patch to fix this segfault on exit

This patch was created by Thomas Tsai. After this patch was applied, the issue has gone.

The attached patch looks good from a quick glance, however
as I cannot reproduce the issue for now I cannot judge if the
patch should really be applied or I cannot judge if this
issue (if exists) is a bug on jfbterm side.

Would you attach a backtrace from gdm for this issue if possible?

(In reply to comment #4)
> 
> Would you attach a backtrace from gdm for this issue if possible?

s/gdm/gdb

Created attachment 495144 [details]
gdb and new patch for tterm_reset_utmp function

Hello,

I think the segfault is happening with incorrect ut_id value. I tried to fix ut_id, but it works not well especially on Debian for some unknown reason. After some test, I also found some ut_id is empty, so the new patch replace ut_id with ut_line and therefore return code checking is not necessary. Just in caes, we still keep it.

I attached segfault log, new patch and fixed log. There is utmp structure value dump from gdb, just compare utmp and utp to confirm the bug issue.

BR, Thomas.

---- summary of error log dump ----
(gdb) p utmp
$1 = {ut_type = 7, ut_pid = 0, ut_line = '\000' <repeats 31 times>, ut_i
d = "\000\000\000",
  ut_user = '\000' <repeats 31 times>, ut_host = '\000' <repeats 255 times>, ut_exit = {
    e_termination = 0, e_exit = 0}, ut_session = 0, ut_tv = {tv_sec = 0, tv_usec = 0},
  ut_addr_v6 = {0, 0, 0, 0}, __unused = '\000' <repeats 19 times>}
(gdb) p *utp
Cannot access memory at address 0x0
(gdb) c
Continuing.

Program received signal SIGSEGV, Segmentation fault.
tterm_reset_utmp (p=<value optimized out>) at term.c:309
---- done ----

--- after new patch ----
(gdb) p utmp
$1 = {ut_type = 7, ut_pid = 0, ut_line = "pts/3", '\000' <repeats 26 times>,
  ut_id = "\000\000\000", ut_user = '\000' <repeats 31 times>,
  ut_host = '\000' <repeats 255 times>, ut_exit = {e_termination = 0, e_exit = 0}, ut_session = 0,
  ut_tv = {tv_sec = 0, tv_usec = 0}, ut_addr_v6 = {0, 0, 0, 0},
  __unused = '\000' <repeats 19 times>}
(gdb) p *utp
$2 = {ut_type = 7, ut_pid = 3842, ut_line = "pts/3", '\000' <repeats 26 times>,
  ut_id = "\000\000\000", ut_user = "root", '\000' <repeats 27 times>,
  ut_host = '\000' <repeats 255 times>, ut_exit = {e_termination = 0, e_exit = 0}, ut_session = 0,
  ut_tv = {tv_sec = 1303916197, tv_usec = 0}, ut_addr_v6 = {0, 0, 0, 0},
  __unused = '\000' <repeats 19 times>}
(gdb) c
Continuing.

Breakpoint 1, tterm_reset_utmp (p=0x8058c40) at term.c:298
298     {
(gdb) c
Continuing.

Program exited normally.
---- done ----

Well, now I am trying slowly to understand what tterm_reset_utmp() in term.c is doing, however, $ man getutid says:

    If ut->ut_type is one of INIT_PROCESS, LOGIN_PROCESS, 
    USER_PROCESS, or DEAD_PROCESS, getutid() will find the 
    first entry  whose  ut_id  field matches ut->ut_id.

So changing strncpy(utmp.ut_id, tn, sizeof(utmp.ut_id)); to
strncpy(utmp.ut_line, tn, sizeof(utmp.ut_line)); before calling 
utp = getutid(&utmp); seems wrong to me: As memset is called beforehand, after your patch is applied utmp.ut_id will be "\0" when getutid(&utmp) is called, and utp will be always NULL (perhaps).

Thomas, if possible would you dump the contents of TTerm* p on tterm_reset_utmp when this issue happens (especially p->name)?

Hello,

The TTerm values dump here:
$3 = (TTerm *) 0x8058c40
(gdb) p *p
$4 = {ptyfd = 9, ttyfd = 10, name = "/dev/pts/2", '\000' <repeats 53 times>, 
,.......(skip)
(gdb) p p->name
$5 = "/dev/pts/2", '\000' <repeats 53 times>

The correct ut_id should be '/2', and 'rindex(...)+4' may get incorrect ut_id.
We could still try to back to fix ut_id, or replace getutid(&utmp) with getutline(&utmp).

If you need more information, I am happy to dump it for you.
Thank You.

BR, Thomas.

Okay, thank you for followups. I will submit possible fix for this issue soon. I would appreciate it if you would try the new rpms which may fix this issue.

jfbterm-0.4.7-25.fc15 has been submitted as an update for Fedora 15.
https://admin.fedoraproject.org/updates/jfbterm-0.4.7-25.fc15

jfbterm-0.4.7-25.fc14 has been submitted as an update for Fedora 14.
https://admin.fedoraproject.org/updates/jfbterm-0.4.7-25.fc14

jfbterm-0.4.7-25.fc13 has been submitted as an update for Fedora 13.
https://admin.fedoraproject.org/updates/jfbterm-0.4.7-25.fc13

I confirmed that this patch fixes the issue.
I have downloaded 
http://kojipkgs.fedoraproject.org/packages/jfbterm/0.4.7/25.fc14/i686/jfbterm-0.4.7-25.fc14.i686.rpm
and install it on an i686 Fedora 14 machine. After that, no segfault issue when jfbterm exists.
Cool!
BTW, do you know anyone in the upstream:
http://sourceforge.jp/projects/jfbterm/memberlist
?
Is that possible all the patches in Fedora can be mergered by the upstream project and have another release (e.g. 0.4.8)? 
Thanks.

Steven.

Package jfbterm-0.4.7-25.fc14:
* should fix your issue,
* was pushed to the Fedora 14 testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=updates-testing jfbterm-0.4.7-25.fc14'
as soon as you are able to.
Please go to the following url:
https://admin.fedoraproject.org/updates/jfbterm-0.4.7-25.fc14
then log in and leave karma (feedback).

jfbterm-0.4.7-25.fc15 has been pushed to the Fedora 15 stable repository.  If problems still persist, please make note of it in this bug report.

jfbterm-0.4.7-25.fc14 has been pushed to the Fedora 14 stable repository.  If problems still persist, please make note of it in this bug report.

jfbterm-0.4.7-25.fc13 has been pushed to the Fedora 13 stable repository.  If problems still persist, please make note of it in this bug report.