Bug 132845 - [geom] [patch] ggated(8) does not close files opened after disconnect
Summary: [geom] [patch] ggated(8) does not close files opened after disconnect
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: 11.2-RELEASE
Hardware: Any Any
: Normal Affects Only Me
Assignee: Mark Johnston
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-03-20 08:10 UTC by ota
Modified: 2020-09-07 23:23 UTC (History)
3 users (show)

See Also:
linimon: mfc-stable12?


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description ota 2009-03-20 08:10:03 UTC
ggated doesn't close local file after closing client connections.

By the way, the patch at http://www.freebsd.org/cgi/query-pr.cgi?pr=bin/132798 also includes this fix, too.  I decided to file this because it is a separate issue.

Fix: 

Add the following in connection_remove function in sbin/ggate/ggated/ggated.c

        if(conn->c_diskfd == -1)
                close(conn->c_diskfd);


FYI:
static void
connection_remove(struct ggd_connection *conn)
{

        LIST_REMOVE(conn, c_next);
        g_gate_log(LOG_DEBUG, "Connection removed [%s %s].",
            ip2str((struct sockaddr*)&conn->c_srcaddr), conn->c_path);
        if (conn->c_sendfd != -1)
                close(conn->c_sendfd);
        if (conn->c_recvfd != -1)
                close(conn->c_recvfd);
        free(conn->c_path);
        free(conn);
}
How-To-Repeat: server# ggated
client# ggatec create -oro server /dev/da0
client# ggatec destroy -u 0
server# mount -orw /dev/da0 /mnt/backup

RW mount on server fails because ggated remain /dev/da0 opened.
Comment 1 Mark Linimon freebsd_committer freebsd_triage 2009-03-20 19:08:18 UTC
Responsible Changed
From-To: freebsd-bugs->freebsd-geom

Over to maintainer(s).
Comment 2 ota 2016-02-05 05:08:25 UTC
This has been quite a while and I happened to open this after seeing an update.

Looking at the report myself and ggated, ggated grabs all files in the export file at start time.  My report indicates that a file was still locked while ggated was alive although a client disconnected.

It doesn't sound like a bug but rather feature to keep files locked while the daemon is running.
Comment 3 Allan Jude freebsd_committer 2016-02-05 05:26:04 UTC
From my look at it, it looks like ggated opens the device twice when the client connects.

When I repeatedly cycled the client, it seemed to leak 1 GEOM reference each time.

I will write up some reproduction steps over the weekend
Comment 4 Fabian Keil 2016-02-05 07:50:57 UTC
Multiple ggated file descriptor leaks have been fixed in ElectroBSD and reported to the FreeBSD SO in 2014 and 2015 because of the DoS aspect.

For detail see:
https://www.fabiankeil.de/sourcecode/electrobsd/ElectroBSD-r295083-0c7f4d6.diff
and grep for ggate.
Comment 5 Eitan Adler freebsd_committer freebsd_triage 2018-05-28 19:46:10 UTC
batch change:

For bugs that match the following
-  Status Is In progress 
AND
- Untouched since 2018-01-01.
AND
- Affects Base System OR Documentation

DO:

Reset to open status.


Note:
I did a quick pass but if you are getting this email it might be worthwhile to double check to see if this bug ought to be closed.
Comment 6 ota 2020-08-24 02:06:07 UTC
This is a long standing issue.
I've even forgotten I had reported more than a decade ago.

I re-discovered it after losing a local patch due to some conflicts.
Comment 7 ota 2020-08-24 02:17:30 UTC
https://reviews.freebsd.org/D26168 for another attempt to fix the bug.
Comment 8 commit-hook freebsd_committer 2020-08-31 16:00:04 UTC
A commit references this bug:

Author: markj
Date: Mon Aug 31 15:59:17 UTC 2020
New revision: 364995
URL: https://svnweb.freebsd.org/changeset/base/364995

Log:
  ggated(8): Avoid doubly opening the requested disk device.

  - Initialize the disk device fd field in connection_new().
  - Close the disk device after handing the connection over
    to a child worker.
  - Avoid re-opening a disk device for each connection from
    the same client, avoiding an fd leak.

  PR:		132845
  Submitted by:	Yoshihiro Ota <ota@j.email.ne.jp>
  MFC after:	1 week
  Differential Revision:	https://reviews.freebsd.org/D26168

Changes:
  head/sbin/ggate/ggated/ggated.c
Comment 9 commit-hook freebsd_committer 2020-09-07 23:23:02 UTC
A commit references this bug:

Author: markj
Date: Mon Sep  7 23:22:16 UTC 2020
New revision: 365436
URL: https://svnweb.freebsd.org/changeset/base/365436

Log:
  MFC r364995:
  ggated(8): Avoid doubly opening the requested disk device.

  PR:	132845

Changes:
_U  stable/12/
  stable/12/sbin/ggate/ggated/ggated.c