Skip to content

Commit ea98215

Browse files
kerumetogvisor-bot
kerumeto
authored and
gvisor-bot
committed
Update TODO bugs.
PiperOrigin-RevId: 790823716
1 parent 67b57f6 commit ea98215

File tree

4 files changed

+26
-29
lines changed

4 files changed

+26
-29
lines changed

‎pkg/sentry/socket/netlink/netfilter/protocol.go‎

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ func (p *Protocol) ProcessMessage(ctx context.Context, s *netlink.Socket, msg *n
165165

166166
// newTable creates a new table for the given family.
167167
func (p *Protocol) newTable(nft *nftables.NFTables, attrs map[uint16]nlmsg.BytesView, family stack.AddressFamily, flags uint16, ms *nlmsg.MessageSet) *syserr.AnnotatedError {
168-
// TODO: b/421437663 - Handle the case where the table name is set to empty string.
168+
// TODO: b/434242152 - Handle the case where the table name is set to empty string.
169169
// The table name is required.
170170
tabNameBytes, ok := attrs[linux.NFTA_TABLE_NAME]
171171
if !ok {
@@ -192,7 +192,7 @@ func (p *Protocol) newTable(nft *nftables.NFTables, attrs map[uint16]nlmsg.Bytes
192192
return p.updateTable(nft, tab, attrs, family, ms)
193193
}
194194

195-
// TODO: b/421437663 - Support additional user-specified table flags.
195+
// TODO: b/434242152 - Support additional user-specified table flags.
196196
var attrFlags uint32 = 0
197197
if uflags, ok := attrs[linux.NFTA_TABLE_FLAGS]; ok {
198198
attrFlags, _ = uflags.Uint32()
@@ -262,7 +262,7 @@ func (p *Protocol) updateTable(nft *nftables.NFTables, tab *nftables.Table, attr
262262
// getTable returns a table for the given family.
263263
func (p *Protocol) getTable(nft *nftables.NFTables, attrs map[uint16]nlmsg.BytesView, family stack.AddressFamily, msgFlags uint16, ms *nlmsg.MessageSet) *syserr.AnnotatedError {
264264
if (msgFlags & linux.NLM_F_DUMP) != 0 {
265-
// TODO: b/421437663 - Support dump requests for tables.
265+
// TODO: b/434242152 - Support dump requests for tables.
266266
return syserr.NewAnnotatedError(syserr.ErrNotSupported, fmt.Sprintf("Nftables: Table dump is not currently supported"))
267267
}
268268

@@ -423,7 +423,7 @@ func (p *Protocol) newChain(nft *nftables.NFTables, attrs map[uint16]nlmsg.Bytes
423423
return syserr.NewAnnotatedError(syserr.ErrNotSupported, fmt.Sprintf("Nftables: Chain with handle: %d already exists and NLM_F_REPLACE is not supported", chain.GetHandle()))
424424
}
425425

426-
// TODO: b/421437663: Support updating existing chains.
426+
// TODO: b/434243967: Support updating existing chains.
427427
return syserr.NewAnnotatedError(syserr.ErrNotSupported, fmt.Sprintf("Nftables: Chain flags attribute is not supported for existing chains"))
428428
}
429429

@@ -470,7 +470,7 @@ func (p *Protocol) addChain(attrs map[uint16]nlmsg.BytesView, tab *nftables.Tabl
470470
if err != nil {
471471
return err
472472
}
473-
// TODO: b/421437663 - support NFTA_CHAIN_COUNTERS (nested attribute)
473+
// TODO: b/434243967 - support NFTA_CHAIN_COUNTERS (nested attribute)
474474
if _, ok := attrs[linux.NFTA_CHAIN_COUNTERS]; ok {
475475
return syserr.NewAnnotatedError(syserr.ErrNotSupported, fmt.Sprintf("Nftables: Chain counters attribute is currently not supported"))
476476
}
@@ -526,7 +526,7 @@ func (p *Protocol) chainParseHook(chain *nftables.Chain, family stack.AddressFam
526526
}
527527

528528
if chain != nil {
529-
// TODO: b/421437663 - Support updating existing chains.
529+
// TODO: b/434243967 - Support updating existing chains.
530530
return nil, syserr.NewAnnotatedError(syserr.ErrNotSupported, fmt.Sprintf("Nftables: Updating hook attributes are not supported for existing chains"))
531531
}
532532

@@ -555,7 +555,7 @@ func (p *Protocol) chainParseHook(chain *nftables.Chain, family stack.AddressFam
555555
hookInfo.ChainType = nftables.BaseChainTypeFilter
556556

557557
if chainTypeBytes, ok := hookAttrs[linux.NFTA_CHAIN_TYPE]; ok {
558-
// TODO - b/421437663: Support base chain types other than filter.
558+
// TODO - b/434243967: Support base chain types other than filter.
559559
switch chainType := chainTypeBytes.String(); chainType {
560560
case "filter":
561561
hookInfo.ChainType = nftables.BaseChainTypeFilter
@@ -580,7 +580,7 @@ func (p *Protocol) chainParseHook(chain *nftables.Chain, family stack.AddressFam
580580

581581
var netDevName string
582582
if isNetDevHook(family, hookInfo.HookNum) {
583-
// TODO: b/421437663 - Support chains for the netdev family.
583+
// TODO: b/434243967 - Support chains for the netdev family.
584584
return nil, syserr.NewAnnotatedError(syserr.ErrNotSupported, fmt.Sprintf("Nftables: Netdev hooks are not currently supported"))
585585
}
586586

@@ -607,7 +607,7 @@ func (p *Protocol) chainParseHook(chain *nftables.Chain, family stack.AddressFam
607607
// getChain returns the chain with the given name and table name.
608608
func (p *Protocol) getChain(nft *nftables.NFTables, attrs map[uint16]nlmsg.BytesView, family stack.AddressFamily, msgFlags uint16, ms *nlmsg.MessageSet) *syserr.AnnotatedError {
609609
if (msgFlags & linux.NLM_F_DUMP) != 0 {
610-
// TODO: b/421437663 - Support dump requests for chains.
610+
// TODO: b/434243967 - Support dump requests for chains.
611611
return syserr.NewAnnotatedError(syserr.ErrNotSupported, fmt.Sprintf("Nftables: Chain dump is not currently supported"))
612612
}
613613

@@ -702,7 +702,7 @@ func (p *Protocol) deleteChain(nft *nftables.NFTables, attrs map[uint16]nlmsg.By
702702
}
703703

704704
if chain.IsBaseChain() {
705-
// TODO: b/421437663 - Support deleting netdev basechains.
705+
// TODO: b/434243967 - Support deleting netdev basechains.
706706
if isNetDevHook(family, chain.GetBaseChainInfo().LinuxHookNum) {
707707
return syserr.NewAnnotatedError(syserr.ErrNotSupported, fmt.Sprintf("Nftables: Netdev basechains or basechains attached to Ingress or Egress are not currently supported for deleting"))
708708
}
@@ -713,7 +713,7 @@ func (p *Protocol) deleteChain(nft *nftables.NFTables, attrs map[uint16]nlmsg.By
713713
return syserr.NewAnnotatedError(syserr.ErrBusy, fmt.Sprintf("Nftables: Non-recursive delete on a chain with use > 0 is not supported. Chain %s has chain use %d", chain.GetName(), chain.GetChainUse()))
714714
}
715715

716-
// TODO: b/421437663 - Support iteratively deleting rules in a chain to then
716+
// TODO: b/434243967 - Support iteratively deleting rules in a chain to then
717717
// delete chains. After deleting all the possible rules, if the chain is
718718
// still in use, it cannot be deleted.
719719
if chain.GetChainUse() != 0 {
@@ -752,7 +752,7 @@ func (p *Protocol) newRule(nft *nftables.NFTables, attrs map[uint16]nlmsg.BytesV
752752
return err
753753
}
754754
} else if _, ok := attrs[linux.NFTA_RULE_CHAIN_ID]; ok {
755-
// TODO - b/421437663: Support looking up chains via their transaction id.
755+
// TODO - b/434244017: Support looking up chains via their transaction id.
756756
// This has to do with Linux's transaction system for committing tables
757757
// atomically. This allows users to modify chains that have not yet been
758758
// committed, but given that we do not have a transaction system (tables
@@ -808,7 +808,7 @@ func (p *Protocol) newRule(nft *nftables.NFTables, attrs map[uint16]nlmsg.BytesV
808808
return err
809809
}
810810
} else if _, ok := attrs[linux.NFTA_RULE_POSITION_ID]; ok {
811-
// TODO - b/421437663: Support looking up rules via their position id.
811+
// TODO - b/434244017: Support looking up rules via their position id.
812812
// ID is used for Linux's transaction system like stated above.
813813
return syserr.NewAnnotatedError(syserr.ErrNotSupported, "Nftables: Rule position id is not supported.")
814814
}
@@ -823,7 +823,7 @@ func (p *Protocol) newRule(nft *nftables.NFTables, attrs map[uint16]nlmsg.BytesV
823823
}
824824

825825
rule := &nftables.Rule{}
826-
// TODO: b/421437663 - Support error-checking the size of the expressions.
826+
// TODO: b/434244017 - Support error-checking the size of the expressions.
827827
if udataBytes, ok := attrs[linux.NFTA_RULE_USERDATA]; ok {
828828
if err := rule.SetUserData(udataBytes); err != nil {
829829
return err
@@ -832,7 +832,7 @@ func (p *Protocol) newRule(nft *nftables.NFTables, attrs map[uint16]nlmsg.BytesV
832832

833833
for _, exprInfo := range exprInfos {
834834
err = rule.AddOpFromExprInfo(tab, exprInfo)
835-
// TODO - b/421437663: Create a copy of nftables structure when modifying the table.
835+
// TODO - b/434244017: Create a copy of nftables structure when modifying the table.
836836
// Because we will create a copy of the table, no cleanup is necessary on the error case.
837837
// The table will simply be reverted to the original state.
838838
if err != nil {
@@ -848,7 +848,7 @@ func (p *Protocol) newRule(nft *nftables.NFTables, attrs map[uint16]nlmsg.BytesV
848848
return syserr.NewAnnotatedError(syserr.ErrTooManyOpenFiles, fmt.Sprintf("Nftables: Chain %s has the maximum chain use value at %d", chain.GetName(), chain.GetChainUse()))
849849
}
850850

851-
// TODO - b/421437663: Support replace operations on rules.
851+
// TODO - b/434244017: Support replace operations on rules.
852852
if msgFlags&linux.NLM_F_REPLACE != 0 {
853853
return syserr.NewAnnotatedError(syserr.ErrNotSupported, "Nftables: Replace operations are not currently supported.")
854854
}
@@ -874,7 +874,7 @@ func (p *Protocol) newRule(nft *nftables.NFTables, attrs map[uint16]nlmsg.BytesV
874874
return err
875875
}
876876

877-
// TODO - b/421437663: Support validating the entire table before returning.
877+
// TODO - b/434244017: Support validating the entire table before returning.
878878
return nil
879879
}
880880

‎pkg/tcpip/nftables/nft_immediate.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ func initImmediate(tab *Table, exprInfo ExprInfo) (*immediate, *syserr.Annotated
7777

7878
switch int32(dreg) {
7979
case linux.NFT_JUMP, linux.NFT_GOTO:
80-
// TODO - b/421437663: Add support for jump and goto verdicts.
80+
// TODO - b/434244017: Add support for jump and goto verdicts.
8181
return nil, syserr.NewAnnotatedError(syserr.ErrNotSupported, "Nftables: Verdicts with jump or goto codes are not yet supported")
8282
}
8383
return newImmediate(dreg, regData)

‎pkg/tcpip/nftables/nftables.go‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,6 @@ import (
2828
"gvisor.dev/gvisor/pkg/tcpip/stack"
2929
)
3030

31-
// TODO: b/421437663 - Refactor functions to return a POSIX syserr
32-
3331
//
3432
// Interface-Related Methods
3533
//
@@ -858,7 +856,7 @@ func (c *Chain) SetComment(comment string) {
858856
// - All jump and goto operations have a valid target chain.
859857
// - Loop checking for jump and goto operations.
860858
// - TODO(b/345684870): Add more checks as more operations are supported.
861-
// TODO - b/421437663: Update rules to be in a linked list for faster insertion and deletion.
859+
// TODO - b/434244017: Update rules to be in a linked list for faster insertion and deletion.
862860
func (c *Chain) RegisterRule(rule *Rule, index int) *syserr.AnnotatedError {
863861
// Error checks like these are not part of the nf_tables_api.c. Rather they are error
864862
// checked here for completeness for unit tests. Netfilter sockets should never attempt to register
@@ -920,9 +918,8 @@ func (c *Chain) RegisterAfterExistingRule(newRule *Rule, oldRule *Rule) *syserr.
920918
}
921919

922920
// UnregisterRuleByIndex removes the rule at the given index from the chain's rule list
923-
// and unassigns the chain from the rule then returns the unregistered rule.
921+
// and un-assigns the chain from the rule then returns the unregistered rule.
924922
// Valid indices are -1 (pop) and [0, len-1]. Errors on invalid index.
925-
// TODO: b/421437663 - Need to refactor or implement a function to remove by rule name.
926923
func (c *Chain) UnregisterRuleByIndex(index int) (*Rule, *syserr.AnnotatedError) {
927924
rule, err := c.GetRule(index)
928925
if err != nil {
@@ -1065,7 +1062,7 @@ func (r *Rule) addOperation(op operation) *syserr.AnnotatedError {
10651062
// AddOpFromExprInfo adds an operation to the rule given the expression information.
10661063
func (r *Rule) AddOpFromExprInfo(tab *Table, exprInfo ExprInfo) *syserr.AnnotatedError {
10671064
// Centralized here so that operations can do their own validation when being created.
1068-
// TODO - b/421437663: Support parsing expression types other than NFT_IMMEDIATE
1065+
// TODO - b/434244017: Support parsing expression types other than NFT_IMMEDIATE
10691066
var op operation
10701067
var err *syserr.AnnotatedError
10711068
switch exprInfo.ExprName {

‎pkg/tcpip/nftables/nftables_types.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -964,7 +964,7 @@ func nftDataInit(tab *Table, regType uint32, dataBytes nlmsg.AttrsView) (registe
964964
return nil, syserr.NewAnnotatedError(syserr.ErrInvalidArgument, fmt.Sprintf("Nftables: Attribute NFTA_DATA_VALUE is not supported for register type %d", regType))
965965
}
966966

967-
// TODO - b/421437663: Add stricter validation for value bytes.
967+
// TODO - b/434244017: Add stricter validation for value bytes.
968968
return newBytesData(valueBytes), nil
969969
} else if vBytes, ok := dataAttrs[linux.NFTA_DATA_VERDICT]; ok {
970970
// Represents a verdict like NF_DROP or NF_ACCEPT.
@@ -1021,7 +1021,7 @@ func nftValidateRegister(reg uint32, regType uint32, data registerData) (uint8,
10211021
return 0, syserr.NewAnnotatedError(syserr.ErrInvalidArgument, fmt.Sprintf("Nftables: Register data is not a verdict data"))
10221022
}
10231023

1024-
// TODO - b/421437663: Add insertion-time validation of chains for jump and goto verdicts.
1024+
// TODO - b/434244017: Add insertion-time validation of chains for jump and goto verdicts.
10251025
if int32(verdictData.data.Code) == linux.NFT_GOTO || int32(verdictData.data.Code) == linux.NFT_JUMP {
10261026
return 0, syserr.NewAnnotatedError(syserr.ErrNotSupported, fmt.Sprintf("Nftables: Verdicts with jump or goto codes are not yet supported"))
10271027
}
@@ -1034,7 +1034,7 @@ func nftValidateRegister(reg uint32, regType uint32, data registerData) (uint8,
10341034
return 0, syserr.NewAnnotatedError(syserr.ErrInvalidArgument, fmt.Sprintf("Nftables: Register %d with type %d is less than %d bytes", reg, regType, linux.NFT_REG_1*linux.NFT_REG_SIZE/linux.NFT_REG32_SIZE))
10351035
}
10361036

1037-
// TODO - b/421437663: Add error checking for the length of the expression data, ensuring it
1037+
// TODO - b/434244017: Add error checking for the length of the expression data, ensuring it
10381038
// can fit within the specified register.
10391039
}
10401040

@@ -1071,7 +1071,7 @@ func validateVerdictData(tab *Table, bytes nlmsg.AttrsView) (stack.NFVerdict, *s
10711071
return v, err
10721072
}
10731073
} else if _, ok := verdictAttrs[linux.NFTA_VERDICT_CHAIN_ID]; ok {
1074-
// TODO - b/421437663: Add support for looking up chains via their transaction id.
1074+
// TODO - b/434243967: Add support for looking up chains via their transaction id.
10751075
return v, syserr.NewAnnotatedError(syserr.ErrNotSupported, fmt.Sprintf("Nftables: Looking up chains via their id is not supported"))
10761076
} else {
10771077
return v, syserr.NewAnnotatedError(syserr.ErrInvalidArgument, fmt.Sprintf("Nftables: Attributes for verdict data must contain a chain name or chain id"))
@@ -1098,7 +1098,7 @@ func validateVerdictData(tab *Table, bytes nlmsg.AttrsView) (stack.NFVerdict, *s
10981098
return v, syserr.NewAnnotatedError(syserr.ErrInvalidArgument, fmt.Sprintf("Nftables: Unsupported verdict code: %d", verdictCode))
10991099
}
11001100

1101-
// TODO - b/421437663: Potential modify this to take a pointer to the chain it is jumping to.
1101+
// TODO - b/345684870: Potentially modify this to take a pointer to the chain it is jumping to.
11021102
// Would need to ensure that the chain cannot be removed while it is being pointed to (using use field).
11031103
v.Code = verdictCode
11041104
return v, nil

0 commit comments

Comments
 (0)