Re: Re: Fix resource leak in iptables/xtables-restore.c

[周恺航 <22321077@xxxxxxxxxx>]

> -----Original Message-----


> From: "Phil Sutter" <phil@xxxxxx>

> Sent: Monday, May 12, 2025 18:23:28
> To: 周恺航 <22321077@xxxxxxxxxx>
> Cc: netfilter-devel@xxxxxxxxxxxxxxx
> Subject: Re: Fix resource leak in iptables/xtables-restore.cc
> 
> Hi,
> 
> On Mon, May 12, 2025 at 03:10:47PM +0800, 周恺航 wrote:
> > The function xtables_restore_main opens a file stream p.in but fails to close it before returning. This leads to a resource leak as the file descriptor remains open.
> > 
> > 
> > Signed-off-by: Kaihang Zhou <22321077@xxxxxxxxxx>
> > 
> > ---
> >  iptables/xtables-restore.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > 
> > diff --git a/iptables/xtables-restore.c b/iptables/xtables-restore.c
> > 
> > index e7802b9e..f09ab7ee 100644
> > --- a/iptables/xtables-restore.c
> > +++ b/iptables/xtables-restore.c
> > @@ -381,6 +381,7 @@ xtables_restore_main(int family, const char *progname, int argc, char *argv[])
> >                 break;
> >         default:
> >                 fprintf(stderr, "Unknown family %d\n", family);
> > +               fclose(p.in);
> >                 return 1;
> >         }
> 
> Since this is not the only error path which leaves p.in open (eight
> lines below is the next one for instance), why fix this one in
> particular and leave the other ones in place?
> 
> Cheers, Phil

At first, I thought that not closing the file handle before the return was more serious, and that when exit terminates the program, the system might automatically reclaim resources. But it's obvious that this understanding is wrong. Both are bad programming habits and may lead to problems in resource management and program stability. I've revised the patch.Thank you.



Signed-off-by: Kaihang Zhou <22321077@xxxxxxxxxx>


---

 iptables/xtables-restore.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/iptables/xtables-restore.c b/iptables/xtables-restore.c


index e7802b9e..62ee68fc 100644

--- a/iptables/xtables-restore.c
+++ b/iptables/xtables-restore.c
@@ -381,6 +381,7 @@ xtables_restore_main(int family, const char *progname, int argc, char *argv[])
                break;
        default:
                fprintf(stderr, "Unknown family %d\n", family);
+               fclose(p.in);
                return 1;
        }

@@ -389,6 +390,7 @@ xtables_restore_main(int family, const char *progname, int argc, char *argv[])


                                xtables_globals.program_name,

                                xtables_globals.program_version,
                                strerror(errno));
+               fclose(p.in);
                exit(EXIT_FAILURE);
        }
        h.noflush = noflush;


------------------------------
2.43.0